This is an archive of the discontinued LLVM Phabricator instance.

Stripping invariant.group metadata in CodeGenPrepare
ClosedPublic

Authored by Prazek on Sep 14 2015, 11:24 PM.

Details

Diff Detail

Event Timeline

Prazek updated this revision to Diff 34784.Sep 14 2015, 11:24 PM
Prazek retitled this revision from to Stripping invariant.group metadata in CodeGenPrepare.
Prazek updated this object.
Prazek added reviewers: nlewycky, rsmith, majnemer.
Prazek added a subscriber: llvm-commits.
nlewycky accepted this revision.Sep 15 2015, 9:49 AM
nlewycky edited edge metadata.

LGTM

test/Transforms/CodeGenPrepare/invariant.group.ll
5–10

Please interleave these in the def'n of @foo.

This revision is now accepted and ready to land.Sep 15 2015, 9:49 AM
Prazek updated this revision to Diff 34814.Sep 15 2015, 11:10 AM
Prazek edited edge metadata.
Prazek closed this revision.Sep 15 2015, 11:35 AM
Prazek marked an inline comment as done.
reames added a subscriber: reames.Sep 16 2015, 8:20 PM

Can you give some context for *why* this is needed? It doesn't seem
entirely obvious, isn't in the change description, and doesn't appear to
be in comments...

Philip

Because I remove all invariant.group.barrier's in CodeGenPrepare, I also want to remove invariant.group metadata. After removing all invariant.barriers it would be invalid to optimize based on invariant.group metadata, so I don't want to leave trap for future programmer.

Ok, that helps, but *why* are you removing all the
invariant.group.barriers? Is that just an implementation detail? Or is
it something fundamental? In either case, the code in CGP should
include comments which clarify this.

Philip

Because that what CodeGenPrepare is for right?
Because invariant.group.barrier is just intrinsic for optimizations purposes (like most of them if not all) we don't want to have calls to intrinsics functions in binaries,
so we remove all intrinsics in CodeGenPrepare.

But I will add small comment near line 1411 when things happen