[PATCH] D69477: [InstCombine] keep assumption before sinking calls

Tyker via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 29 18:10:11 PDT 2019


Tyker updated this revision to Diff 226996.
Tyker marked 8 inline comments as done.
Tyker added a comment.

fixed comments from @jdoerfert

for the choice between the assume or not sinking the call, both have there drawback. if no users from the call can benefit from the nonnull (or other attributes) grantee sinking would be the best choice. but in the given example not sinking is the best. i tried to get the best of both with the assume, but it does add a use which can prevent other optimization. i don't know which is the best short term solution.

i completely agree with your points on a good middle-long term solution. but a good solution can't be implemented quickly. when thinking about it there is many situation were we lose information by performing a transformation which can worsen other optimizations.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69477/new/

https://reviews.llvm.org/D69477

Files:
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/test/Transforms/InstCombine/Assume-Remplacing-Call.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69477.226996.patch
Type: text/x-patch
Size: 8698 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191030/305fa3aa/attachment.bin>


More information about the llvm-commits mailing list