[PATCH] D39835: [GVN PRE] Clear nsw/nuw for original values in LoadPRE

Serguei Katkov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 9 19:14:06 PST 2017


skatkov added a comment.

I tend to agree with Eli that we do not need this patch but I disagree that the previous patch is not needed.
And there is a between them. And it is actually here:

> The only case which causes a problem is if you take the result of an nsw add, and use it instead of the result of a non-nsw add.

In this case we do not introduce new behavior while preserving the nuw/nsw. Specifically if we went by the path entry->bb->merge we stored nuw/nsw value and load it in merge block. As soon as it is nuw, %cmp will be false.

In pre-jt-add.ll, the situation is different. if we execute the path entry->bb->merge we will use an add without nuw/nsw in cmp instruction but after phi creation without clearing nuw/nsw the same path will result in nsw/nuw add will be used in cmp instruction., So we changed the behavior  specifically we use the result of nuw add instead of non-nuw add. So this is a bug.

If there is an agreement on that I propose to conclude that that patch is valid and this one should be abandoned.

Comments?


https://reviews.llvm.org/D39835





More information about the llvm-commits mailing list