Skip to content

Commit 8a6dae4

Browse files
committed
Some getfield related fixes in inference
* Use the field index passed in in `lift_leaves` The caller has already done all the computation including bound checking. The `field` computed in this function is also affecting all the following iterations which is almost certainly wrong. * Remove unnecessary type check on `field` in `lift_leaves` since it is always `Int` * Move a branch disabling `return nothing` higher up * Remove some duplicated calculation on field index in `getfield_elim_pass!` * Fix `try_compute_fieldidx` to return `nothing` for non-`Int` `Integer` field index. This can cause `getfield_nothrow` to return incorrect result. It also gives the caller worse type info about the return value. * Teach `getfield_nothrow` that `isbits` field cannot be undefined and getfield on such field cannot throw. This is already handled in `isdefined_tfunc`. * Fix a few wrong use of `isbits` in dead branches ---- Ref #26948 (fa02d34) Ref #27126 (9100329)
1 parent 483b637 commit 8a6dae4

File tree

3 files changed

+24
-12
lines changed

3 files changed

+24
-12
lines changed

base/compiler/ssair/passes.jl

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ function lift_leaves(compact::IncrementalCompact, @nospecialize(stmt),
288288
else
289289
def = compact[leaf]
290290
end
291-
if is_tuple_call(compact, def) && isa(field, Int) && 1 <= field < length(def.args)
291+
if is_tuple_call(compact, def) && 1 <= field < length(def.args)
292292
lifted = def.args[1+field]
293293
if is_old(compact, leaf) && isa(lifted, SSAValue)
294294
lifted = OldSSAValue(lifted.id)
@@ -307,8 +307,6 @@ function lift_leaves(compact::IncrementalCompact, @nospecialize(stmt),
307307
end
308308
(isa(typ, DataType) && (!typ.abstract)) || return nothing
309309
@assert !typ.mutable
310-
field = try_compute_fieldidx_expr(typ, stmt)
311-
field === nothing && return nothing
312310
if length(def.args) < 1 + field
313311
ftyp = fieldtype(typ, field)
314312
if !isbitstype(ftyp)
@@ -323,7 +321,7 @@ function lift_leaves(compact::IncrementalCompact, @nospecialize(stmt),
323321
compact[leaf] = nothing
324322
for i = (length(def.args) + 1):(1+field)
325323
ftyp = fieldtype(typ, i - 1)
326-
isbits(ftyp) || return nothing
324+
isbitstype(ftyp) || return nothing
327325
push!(def.args, insert_node!(compact, leaf, result_t, Expr(:new, ftyp)))
328326
end
329327
compact[leaf] = def
@@ -342,22 +340,22 @@ function lift_leaves(compact::IncrementalCompact, @nospecialize(stmt),
342340
else
343341
typ = compact_exprtype(compact, leaf)
344342
if !isa(typ, Const)
343+
# Disabled since #27126
344+
return nothing
345345
# If the leaf is an old ssa value, insert a getfield here
346346
# We will revisit this getfield later when compaction gets
347347
# to the appropriate point.
348348
# N.B.: This can be a bit dangerous because it can lead to
349349
# infinite loops if we accidentally insert a node just ahead
350350
# of where we are
351-
if is_old(compact, leaf) && (isa(field, Int) || isa(field, Symbol))
351+
if is_old(compact, leaf)
352352
(isa(typ, DataType) && (!typ.abstract)) || return nothing
353353
@assert !typ.mutable
354354
# If there's the potential for an undefref error on access, we cannot insert a getfield
355-
if field > typ.ninitialized && !isbits(fieldtype(typ, field))
356-
return nothing
355+
if field > typ.ninitialized && !isbitstype(fieldtype(typ, field))
357356
lifted_leaves[leaf] = RefValue{Any}(insert_node!(compact, leaf, make_MaybeUndef(result_t), Expr(:call, :unchecked_getfield, SSAValue(leaf.id), field), true))
358357
maybe_undef = true
359358
else
360-
return nothing
361359
lifted_leaves[leaf] = RefValue{Any}(insert_node!(compact, leaf, result_t, Expr(:call, getfield, SSAValue(leaf.id), field), true))
362360
end
363361
continue
@@ -671,7 +669,7 @@ function getfield_elim_pass!(ir::IRCode)
671669

672670
isempty(leaves) && continue
673671

674-
field = try_compute_fieldidx_expr(struct_typ, stmt)
672+
field = try_compute_fieldidx(struct_typ, field)
675673
field === nothing && continue
676674

677675
r = lift_leaves(compact, stmt, result_t, field, leaves)
@@ -806,7 +804,7 @@ function getfield_elim_pass!(ir::IRCode)
806804
for stmt in du.uses
807805
ir[SSAValue(stmt)] = compute_value_for_use(ir, domtree, allblocks, du, phinodes, fidx, stmt)
808806
end
809-
if !isbitstype(fieldtype(typ, fidx))
807+
if !isbitstype(ftyp)
810808
for (use, list) in preserve_uses
811809
push!(list, compute_value_for_use(ir, domtree, allblocks, du, phinodes, fidx, use))
812810
end

base/compiler/tfuncs.jl

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,8 @@ function try_compute_fieldidx(typ::DataType, @nospecialize(field))
659659
if isa(field, Symbol)
660660
field = fieldindex(typ, field, false)
661661
field == 0 && return nothing
662-
elseif isa(field, Integer)
662+
elseif isa(field, Int)
663+
# Numerical field name can only be of type `Int`
663664
max_fields = fieldcount_noerror(typ)
664665
max_fields === nothing && return nothing
665666
(1 <= field <= max_fields) || return nothing
@@ -706,7 +707,8 @@ function getfield_nothrow(@nospecialize(s00), @nospecialize(name), @nospecialize
706707
return false
707708
end
708709

709-
s = unwrap_unionall(widenconst(s00))
710+
s0 = widenconst(s00)
711+
s = unwrap_unionall(s0)
710712
if isa(s, Union)
711713
return getfield_nothrow(rewrap(s.a, s00), name, inbounds) &&
712714
getfield_nothrow(rewrap(s.b, s00), name, inbounds)
@@ -723,6 +725,8 @@ function getfield_nothrow(@nospecialize(s00), @nospecialize(name), @nospecialize
723725
field = try_compute_fieldidx(s, name.val)
724726
field === nothing && return false
725727
field <= s.ninitialized && return true
728+
# `try_compute_fieldidx` already check for field index bound.
729+
!isvatuple(s) && isbitstype(fieldtype(s0, field)) && return true
726730
end
727731

728732
return false

test/compiler/irpasses.jl

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,16 @@ let K = rand(2,2)
315315
@test test_29253(K) == 2
316316
end
317317

318+
function no_op_refint(r)
319+
r[]
320+
return
321+
end
322+
let code = code_typed(no_op_refint,Tuple{Base.RefValue{Int}})[1].first.code
323+
@test length(code) == 1
324+
@test isa(code[1], Core.ReturnNode)
325+
@test code[1].val === nothing
326+
end
327+
318328
# check getfield elim handling of GlobalRef
319329
const _some_coeffs = (1,[2],3,4)
320330
splat_from_globalref(x) = (x, _some_coeffs...,)

0 commit comments

Comments
 (0)