EA: perform analysis once for post-optimization IR, and remove IPO EA#51318
EA: perform analysis once for post-optimization IR, and remove IPO EA#51318
IPO EA#51318Conversation
vtjnash
left a comment
There was a problem hiding this comment.
SGTM. But will this introduce any issues with convergence, or is that already addressed by #51092?
Also, do we need to be concerned with this that return_type_tfunc is marked EFFECTS_TOTAL and MethodResultPure, even though it would be a significant soundness issue to run code that contains that call via const-eval?
164d103 to
043b756
Compare
I don't foresee any convergence issues, as EA will be executed on each
This seems like a separate concern? Considering the effects modeling of that function, I'm actually uncertain why it's marked |
d9a52d9 to
c675a80
Compare
I guess that is fine. It adds another reason that the computed results from inference are not consistent, but we already have fundamental reasons of computability that the computed effects cannot be accessed in a consistent-preserving way. (And I maintain my hope that someday we can expose Core.Compiler.return_effect optimization in the same way as Core.Compiler.return_type 😁) |
Following the discussions and changes in #50805, we now consider post-inlining IR as IPO-valid. Revisiting EA, I've realized that running EA twice—once for computing IPO-valid escape cache and once for local optimization analysis—is redundant. This commit streamlines the EA process to perform the analysis just once on post-optimization IR, and caches that result. This change also removes all interprocedural EA code, which had significant overlap with inlining code.
Co-authored-by: Julian Samaroo <jpsamaroo@jpsamaroo.me>
9dd2ed7 to
c3b5bd5
Compare
…A` (#51318) Following the discussions and changes in #50805, we now consider post-inlining IR as IPO-valid. Revisiting EA, I've realized that running EA twice—once for computing IPO-valid escape cache and once for local optimization analysis—is redundant. This commit streamlines the EA process to perform the analysis just once on post-optimization IR, and caches that result. This change also removes all interprocedural EA code, which had significant overlap with inlining code. --------- Co-authored-by: Julian Samaroo <jpsamaroo@jpsamaroo.me>
Following the discussions and changes in #50805, we now consider post-inlining IR as IPO-valid. Revisiting EA, I've realized that running EA twice—once for computing IPO-valid escape cache and once for local optimization analysis—is redundant. This commit streamlines the EA process to perform the analysis just once on post-optimization IR, and caches that result. This change also removes all interprocedural EA code, which had significant overlap with inlining code.