Skip to content

Commit

Permalink
irinterp: Allow setting all IR flags (#48993)
Browse files Browse the repository at this point in the history
Currently, `IR_FLAG_NOTHROW` is the only flag that irinterp is allowed to
set on statements, under the assumption that in order for a call to
be irinterp-eligible, it must have been proven `:foldable`, thus `:effect_free`,
and thus `IR_FLAG_EFFECT_FREE` was assumed to have been set. That reasoning
was sound at the time this code was written, but have since introduced
`EFFECT_FREE_IF_INACCESSIBLEMEMONLY`, which breaks the reasoning that
an `:effect_free` inference for the whole function implies the flag on
every statement. As a result, we were failing to DCE otherwise dead
statements if the IR came from irinterp.
  • Loading branch information
Keno committed Mar 14, 2023
1 parent 6c24103 commit 7ba7e32
Showing 1 changed file with 3 additions and 6 deletions.
9 changes: 3 additions & 6 deletions base/compiler/ssair/irinterp.jl
Original file line number Diff line number Diff line change
Expand Up @@ -238,12 +238,9 @@ function reprocess_instruction!(interp::AbstractInterpreter,
head = inst.head
if head === :call || head === :foreigncall || head === :new || head === :splatnew
(; rt, effects) = abstract_eval_statement_expr(interp, inst, nothing, ir, irsv.mi)
# All other effects already guaranteed effect free by construction
if is_nothrow(effects)
ir.stmts[idx][:flag] |= IR_FLAG_NOTHROW
if isa(rt, Const) && is_inlineable_constant(rt.val)
ir.stmts[idx][:inst] = quoted(rt.val)
end
ir.stmts[idx][:flag] |= flags_for_effects(effects)
if is_foldable(effects) && isa(rt, Const) && is_inlineable_constant(rt.val)
ir.stmts[idx][:inst] = quoted(rt.val)
end
elseif head === :invoke
mi′ = inst.args[1]::MethodInstance
Expand Down

2 comments on commit 7ba7e32

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your package evaluation job has completed - possible new issues were detected.
A full report can be found here.

Please sign in to comment.