[PATCH] D116332: [ConstantFold] Make icmp of gep fold offset based

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:1257
+    // signed.
+    CmpInst::Predicate CmpPred = (CmpInst::Predicate)Predicate;
+    if (Ops0->getType()->isPointerTy() &&
----------------
nikic wrote:
> spatel wrote:
> > Similar to your comment in D116168 - I'm not sure why we specify this parameter as `unsigned` instead of the actual enum. I see that we are not including any headers via ConstantFolding.h, so maybe there's some compile-time justification for it?
> Yeah, I believe the reason for the `unsigned` is that it's impossible to forward-declare a nested type in C++.
> 
> What do you think about the new variant that does a cast right at the start of the function?
Sure, that's better. That means we could replace the checks for EQ/NE around line 1242 with ICmpInst::isEquality(Predicate) as another small cleanup.


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

https://reviews.llvm.org/D116332