Skip to content

Conversation

Keno
Copy link
Member

@Keno Keno commented May 2, 2022

This is consistent with what we would do for a non-constant return.
This helps the compiler dump less code onto LLVM in situations
where there is a big basic block that our constprop can see is
dead. That said, for that particular situation, it might be better
to have a constant propagation pass at the IR level that runs
after inlining. We can revisit that after #44660.

@Keno Keno requested a review from aviatesk May 2, 2022 03:14
@aviatesk aviatesk added the merge me PR is reviewed. Merge when all tests are passing label May 2, 2022
@aviatesk
Copy link
Member

aviatesk commented May 2, 2022

@nanosoldier runbenchmarks("inference", vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

This is consistent with what we would do for a non-constant return.
This helps the compiler dump less code onto LLVM in situations
where there is a big basic block that our constprop can see is
dead. That said, for that particular situation, it might be better
to have a constant propagation pass at the IR level that runs
after inlining. We can revisit that after #44660.
@Keno Keno force-pushed the kf/constpropbottom branch from 0e7f8f3 to 907f4d4 Compare May 2, 2022 08:08
@aviatesk aviatesk merged commit 3d787a7 into master May 2, 2022
@aviatesk aviatesk deleted the kf/constpropbottom branch May 2, 2022 11:14
add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (unimprovable return type)")
if isa(rt, Const)
if result.edge_effects.nothrow !== ALWAYS_TRUE
# Could still be improved to Bottom (or at least could see the effects improved)
Copy link
Member

Choose a reason for hiding this comment

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

This seems specifically the case that function was intended to ignore as a possibility. I think a more correct version of this PR would be removing this function entirely, and remarking here only that we do const_prop as a pre-inlining optimization, so the return result is irrelevant to whether it may improve the quality of the inlined code.

@giordano giordano removed the merge me PR is reviewed. Merge when all tests are passing label May 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants