This is an archive of the discontinued LLVM Phabricator instance.

[GlobalOpt] Extend logic in SRA heuristic to skip stores of initializer.
ClosedPublic

Authored by fhahn on Feb 21 2023, 5:23 AM.

Details

Summary

If all stores only store the initializer value of a global, consider it
as not stored in the heuristic. GlobalOpt will remove such stores later
on.

Depends on D129857.

Diff Detail

Event Timeline

fhahn created this revision.Feb 21 2023, 5:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 21 2023, 5:23 AM
fhahn requested review of this revision.Feb 21 2023, 5:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 21 2023, 5:23 AM
fhahn updated this revision to Diff 501813.Mar 2 2023, 2:29 AM

Rebase and ping :)

nikic added inline comments.Mar 2 2023, 2:56 AM
llvm/lib/Transforms/IPO/GlobalOpt.cpp
400
408

Shouldn't we be comparing with the initializer at the specific offset?

fhahn updated this revision to Diff 502568.Mar 6 2023, 3:09 AM

Update to use ConstantFoldLoadFromConst to check the initializer value in the specifi field.

fhahn updated this revision to Diff 502570.Mar 6 2023, 3:11 AM

Rebase on top of 0ecef88.

fhahn marked 2 inline comments as done.Mar 6 2023, 3:11 AM
fhahn added inline comments.
llvm/lib/Transforms/IPO/GlobalOpt.cpp
400

Done!

408

Yeah, I updated the code to use ConstantFoldLoadFromConst

nikic added inline comments.Mar 6 2023, 3:53 AM
llvm/lib/Transforms/IPO/GlobalOpt.cpp
408

Why can't this reuse the offset calculation from above?

fhahn updated this revision to Diff 502584.Mar 6 2023, 4:23 AM
fhahn marked 2 inline comments as done.

Use existing offset directly.

fhahn marked an inline comment as done.Mar 6 2023, 4:27 AM
fhahn added inline comments.
llvm/lib/Transforms/IPO/GlobalOpt.cpp
408

Missed that, should be updated now, thanks!

nikic accepted this revision.Mar 6 2023, 7:18 AM

LGTM, though I think it might be cleaner if we collected the initializer part during the construction of GlobalPart, rather than calculating it twice, here and during the SRA split.

This revision is now accepted and ready to land.Mar 6 2023, 7:18 AM
This revision was landed with ongoing or failed builds.Mar 7 2023, 2:08 AM
This revision was automatically updated to reflect the committed changes.
fhahn marked an inline comment as done.
fhahn added a comment.Mar 7 2023, 5:12 AM

LGTM, though I think it might be cleaner if we collected the initializer part during the construction of GlobalPart, rather than calculating it twice, here and during the SRA split.

Posted D145490 as follow-up