This is an archive of the discontinued LLVM Phabricator instance.

[CaptureTracking] Handle capturing of launder.invariant.group
ClosedPublic

Authored by Prazek on Apr 29 2017, 1:58 PM.

Details

Summary

launder.invariant.group has the same rules of capturing as
bitcast, gep, etc - the original value is not captured
if the returned pointer is not captured.

With this patch, we mark 40% more functions as noalias when compiling with -fstrict-vtable-pointers;
1078 vs 1778 (39.37%)

Diff Detail

Event Timeline

Prazek created this revision.Apr 29 2017, 1:58 PM
Prazek edited the summary of this revision. (Show Details)Apr 29 2017, 2:20 PM

Is there anyone who could take a look at this small patch? I haven't got any response for almost 3 weeks.

friendly ping42

Not a reviewer, but looks good to me!

Prazek added a comment.Jun 6 2017, 8:42 AM

friendly ping

Prazek updated this revision to Diff 145001.May 3 2018, 4:40 AM
Prazek edited the summary of this revision. (Show Details)

rebase after renaming

Prazek retitled this revision from [CaptureTracking] Handle capturing of invariant.group.barrier to [CaptureTracking] Handle capturing of launder.invariant.group.May 3 2018, 4:41 AM
Prazek edited the summary of this revision. (Show Details)
amharc added a comment.May 3 2018, 4:46 AM

Otherwise looks good to me.

llvm/test/Transforms/FunctionAttrs/nocapture.ll
224

s/nocaptureBarriers/nocaptureLaunder/

xbolva00 accepted this revision.May 3 2018, 6:57 AM
xbolva00 added a subscriber: xbolva00.
This comment was removed by xbolva00.
This revision is now accepted and ready to land.May 3 2018, 6:57 AM
xbolva00 added inline comments.May 3 2018, 6:58 AM
llvm/test/Transforms/FunctionAttrs/nocapture.ll
224

Please fix it and rerun with update_test_checks

xbolva00 requested changes to this revision.May 3 2018, 6:59 AM
This revision now requires changes to proceed.May 3 2018, 6:59 AM
xbolva00 added inline comments.May 3 2018, 7:01 AM
llvm/test/Transforms/FunctionAttrs/nocapture.ll
224

Dont forget to run "ninja check-llvm"

Prazek updated this revision to Diff 145166.May 4 2018, 3:19 AM
Prazek marked 3 inline comments as done.

Fix test

Prazek added inline comments.May 4 2018, 3:20 AM
llvm/test/Transforms/FunctionAttrs/nocapture.ll
224

Yea I know, I was just rebasing in rush.

Prazek marked an inline comment as done.May 4 2018, 3:20 AM
xbolva00 added a subscriber: spatel.EditedMay 4 2018, 3:25 AM

Fix test

Maybe you could even regenerate test file?

./update_test_checks.py ~/LLVM/llvm/test/Transforms/FunctionAttrs/nocapture.ll

Strange, If I did it, i got "WARNING: Found conflicting asm under the same prefix: 'CHECK'!" (@spatel, maybe you can look on it why this warning happens?)

xbolva00 added inline comments.May 4 2018, 3:28 AM
llvm/lib/Analysis/CaptureTracking.cpp
219

is lambda better here? I would prefer seperate function personally.

Prazek added inline comments.May 4 2018, 7:48 PM
llvm/lib/Analysis/CaptureTracking.cpp
219

I prefer lambda because it narrows function visibility to only this function, and capture of worklist and Visited set looks better than passing as a reference.

xbolva00 accepted this revision.May 4 2018, 9:55 PM
This revision is now accepted and ready to land.May 4 2018, 9:55 PM
This revision was automatically updated to reflect the committed changes.
Prazek marked 2 inline comments as done.
Prazek added a comment.May 5 2018, 3:28 AM

Fix test

Maybe you could even regenerate test file?

./update_test_checks.py ~/LLVM/llvm/test/Transforms/FunctionAttrs/nocapture.ll

Strange, If I did it, i got "WARNING: Found conflicting asm under the same prefix: 'CHECK'!" (@spatel, maybe you can look on it why this warning happens?)

After regenerating checks I found out that the autogenerated tests does not actually test what they should. This is because labels does not check function arguments.
This is totally oposite to what this test is doing - it does not modify the actual function IR, but it adds nocapture attributes, so checking function signature is totally sufficient.