[PATCH] D38765: Don't replace constants with constants in GVN

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added a comment.

Suggesting a slightly different fix.



================
Comment at: lib/Transforms/Scalar/GVN.cpp:1366
   }
 
   Constant *True = ConstantInt::getTrue(V->getContext());
----------------
At this point, if V is a constant, you should return false.

There is nothing useful we can do:
There is no equality it is really propagating, since it's not a comparison, and we've already taken care of the case where we have assume(false).

Every other constant is equivalent to assume(true).

That leaves two options:

1. Delete the intrinsic, as it is now pointless.
2. Do nothing here and return false.

Either seems reasonable.

As an aside, it's mentioned this came from a rust issue.
It's probably worth looking to see if something propagated this constant *into* the assume (rather than it being generated by the frontend as an assume of a constant)

If so, *that* should be filed as a bug, as that propagation lost valuable information.




https://reviews.llvm.org/D38765