Page MenuHomePhabricator

Fix Function Attributes computation for llvm.assume

Authored by boazo on May 25 2016, 2:08 AM.



When running the function attributes pass llvm.assume is marked as readnone.
Running instcombine after this pass will remove the llvm.assume causing a potential optimization from happening.
This required after commit rL268068

Diff Detail

Event Timeline

boazo updated this revision to Diff 58390.May 25 2016, 2:08 AM
boazo updated this revision to Diff 58395.
boazo retitled this revision from to Fix Function Attributes computation for llvm.assume.
boazo updated this object.
boazo added reviewers: aaboud, DavidKreitzer, gberry.
boazo added a subscriber: llvm-commits.

whitespace fixes

hfinkel accepted this revision.May 25 2016, 5:02 AM
hfinkel added a reviewer: hfinkel.
hfinkel added a subscriber: hfinkel.


This revision is now accepted and ready to land.May 25 2016, 5:02 AM
majnemer requested changes to this revision.May 25 2016, 10:15 AM
majnemer added a reviewer: majnemer.
majnemer added a subscriber: majnemer.
majnemer added inline comments.

Sentences start with a capital letter and a period.


Why do we need to run InstCombine to test this change?

This revision now requires changes to proceed.May 25 2016, 10:15 AM

@sanjoy Doesn't your intrinsic need the same treatment?

sanjoy accepted this revision.May 25 2016, 11:46 AM
sanjoy added a reviewer: sanjoy.

I think guards are fine since rL268068 only touches assumes. Also guards are not nounwind so even if we do a rL268068 like thing for them, ideally LLVM should not be removing them even after proving readonly (guards are also readonly in a precise sense, not readnone).

pcc added a subscriber: pcc.May 25 2016, 11:53 AM

D20612 solves the same problem in (IMHO) a slightly nicer way.

boazo added a comment.May 26 2016, 7:21 AM

Hi All,

I agree that D20612 solves it better.
Thanks for the review and the fix.
I will abandon this revision.


boazo abandoned this revision.May 26 2016, 7:22 AM