[LoopUnrollAnalyzer] Don't simplify signed pointer comparison

We're generally not able to simplify signed pointer comparisons
(because we don't have no-wrap flags that would permit it), so
we shouldn't pretend that we can in the cost model.

The unsigned comparison case is also not modelled correctly,
as explained in the added comment. As this is a cost model
inaccuracy at worst, I'm leaving it alone for now.

Author: nikic

llvm/lib/Analysis/LoopUnrollAnalyzer.cpp

@@ -155,14 +155,19 @@ bool UnrolledInstAnalyzer::visitCmpInst(CmpInst &I) {
     if (Value *SimpleRHS = SimplifiedValues.lookup(RHS))
       RHS = SimpleRHS;
 
-  if (!isa<Constant>(LHS) && !isa<Constant>(RHS)) {
+  if (!isa<Constant>(LHS) && !isa<Constant>(RHS) && !I.isSigned()) {
     auto SimplifiedLHS = SimplifiedAddresses.find(LHS);
     if (SimplifiedLHS != SimplifiedAddresses.end()) {
       auto SimplifiedRHS = SimplifiedAddresses.find(RHS);
       if (SimplifiedRHS != SimplifiedAddresses.end()) {
         SimplifiedAddress &LHSAddr = SimplifiedLHS->second;
         SimplifiedAddress &RHSAddr = SimplifiedRHS->second;
         if (LHSAddr.Base == RHSAddr.Base) {
+          // FIXME: This is only correct for equality predicates. For
+          // unsigned predicates, this only holds if we have nowrap flags,
+          // which we don't track (for nuw it's valid as-is, for nusw it
+          // requires converting the predicated to signed). As this is used only
+          // for cost modelling, this is not a correctness issue.
           bool Res = ICmpInst::compare(LHSAddr.Offset, RHSAddr.Offset,
                                        I.getPredicate());
           SimplifiedValues[&I] = ConstantInt::getBool(I.getType(), Res);

llvm/unittests/Analysis/UnrollAnalyzerTest.cpp

@@ -221,6 +221,8 @@ TEST(UnrollAnalyzerTest, PtrCmpSimplifications) {
       "  %iv.0 = phi i8* [ %a, %entry ], [ %iv.1, %loop.body ]\n"
       "  %iv2.0 = phi i8* [ %start.iv2, %entry ], [ %iv2.1, %loop.body ]\n"
       "  %cmp = icmp eq i8* %iv2.0, %iv.0\n"
+      "  %cmp2 = icmp slt i8* %iv2.0, %iv.0\n"
+      "  %cmp3 = icmp ult i8* %iv2.0, %iv.0\n"
       "  %iv.1 = getelementptr inbounds i8, i8* %iv.0, i64 1\n"
       "  %iv2.1 = getelementptr inbounds i8, i8* %iv2.0, i64 1\n"
       "  %exitcond = icmp ne i8* %iv.1, %limit\n"
@@ -242,12 +244,21 @@ TEST(UnrollAnalyzerTest, PtrCmpSimplifications) {
 
   BasicBlock::iterator BBI = Header->begin();
   std::advance(BBI, 2);
-  Instruction *Y1 = &*BBI;
+  Instruction *Cmp1 = &*BBI++;
+  Instruction *Cmp2 = &*BBI++;
+  Instruction *Cmp3 = &*BBI++;
   // Check simplification expected on the 5th iteration.
   // Check that "%cmp = icmp eq i8* %iv2.0, %iv.0" is simplified to 0.
-  auto I1 = SimplifiedValuesVector[5].find(Y1);
+  auto I1 = SimplifiedValuesVector[5].find(Cmp1);
   EXPECT_TRUE(I1 != SimplifiedValuesVector[5].end());
   EXPECT_EQ(cast<ConstantInt>((*I1).second)->getZExtValue(), 0U);
+  // Check that "%cmp2 = icmp slt i8* %iv2.0, %iv.0" does not simplify
+  auto I2 = SimplifiedValuesVector[5].find(Cmp2);
+  EXPECT_TRUE(I2 == SimplifiedValuesVector[5].end());
+  // Check that "%cmp3 = icmp ult i8* %iv2.0, %iv.0" is simplified to 0.
+  auto I3 = SimplifiedValuesVector[5].find(Cmp3);
+  EXPECT_TRUE(I3 != SimplifiedValuesVector[5].end());
+  EXPECT_EQ(cast<ConstantInt>((*I1).second)->getZExtValue(), 0U);
 }
 
 TEST(UnrollAnalyzerTest, CastSimplifications) {