Skip to content

Commit 6de98f4

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`. ---- Ref #26948 (fa02d34) Ref #27126 (9100329)
1 parent 483b637 commit 6de98f4

File tree

3 files changed

+19
-8
lines changed

3 files changed

+19
-8
lines changed

base/compiler/ssair/passes.jl

Lines changed: 5 additions & 7 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)
@@ -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
355355
if field > typ.ninitialized && !isbits(fieldtype(typ, field))
356-
return nothing
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)

base/compiler/tfuncs.jl

Lines changed: 4 additions & 1 deletion
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 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
@@ -722,6 +723,8 @@ function getfield_nothrow(@nospecialize(s00), @nospecialize(name), @nospecialize
722723
isa(name, Const) || return false
723724
field = try_compute_fieldidx(s, name.val)
724725
field === nothing && return false
726+
# `try_compute_fieldidx` already check for field index bound.
727+
!isvatuple(s) && isbitstype(fieldtype(s, field)) && return true
725728
field <= s.ninitialized && return true
726729
end
727730

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)