[IRSim] Letting gep instructions be legal for similarity identification.

GetElementPtr instructions require the extra check that all operands
after the first must only be constants and be exactly the same to be
considered similar.

Tests are found in unittests/Analysis/IRSimilarityIdentifierTest.cpp.

llvm/include/llvm/Analysis/IRSimilarityIdentifier.h

@@ -396,10 +396,6 @@ struct IRInstructionMapper {
     // analyzed for similarity as it has no bearing on the outcome of the
     // program.
     InstrType visitDbgInfoIntrinsic(DbgInfoIntrinsic &DII) { return Invisible; }
-    // TODO: Handle GetElementPtrInsts
-    InstrType visitGetElementPtrInst(GetElementPtrInst &GEPI) {
-      return Illegal;
-    }
     // TODO: Handle specific intrinsics.
     InstrType visitIntrinsicInst(IntrinsicInst &II) { return Illegal; }
     // TODO: Handle CallInsts.

llvm/lib/Analysis/IRSimilarityIdentifier.cpp

@@ -83,27 +83,53 @@ bool IRSimilarity::isClose(const IRInstructionData &A,
 
   // Check if we are performing the same sort of operation on the same types
   // but not on the same values.
-  if (A.Inst->isSameOperationAs(B.Inst))
+  if (!A.Inst->isSameOperationAs(B.Inst)) {
-    return true;
+    // If there is a predicate, this means that either there is a swapped
+    // predicate, or that the types are different, we want to make sure that
+    // the predicates are equivalent via swapping.
+    if (isa<CmpInst>(A.Inst) && isa<CmpInst>(B.Inst)) {
 
-  // If there is a predicate, this means that either there is a swapped
+      if (A.getPredicate() != B.getPredicate())
-  // predicate, or that the types are different, we want to make sure that
+        return false;
-  // the predicates are equivalent via swapping.
-  if (isa<CmpInst>(A.Inst) && isa<CmpInst>(B.Inst)) {
 
-    if (A.getPredicate() != B.getPredicate())
+      // If the predicates are the same via swap, make sure that the types are
-      return false;
+      // still the same.
+      auto ZippedTypes = zip(A.OperVals, B.OperVals);
 
-    // If the predicates are the same via swap, make sure that the types are
+      return all_of(
-    // still the same.
+          ZippedTypes, [](std::tuple<llvm::Value *, llvm::Value *> R) {
-    auto ZippedTypes = zip(A.OperVals, B.OperVals);
+            return std::get<0>(R)->getType() == std::get<1>(R)->getType();
+          });
+    }
 
-    return all_of(ZippedTypes, [](std::tuple<llvm::Value *, llvm::Value *> R) {
+    return false;
-      return std::get<0>(R)->getType() == std::get<1>(R)->getType();
-    });
   }
 
-  return false;
+  // Since any GEP Instruction operands after the first operand cannot be
+  // defined by a register, we must make sure that the operands after the first
+  // are the same in the two instructions
+  if (auto *GEP = dyn_cast<GetElementPtrInst>(A.Inst)) {
+    auto *OtherGEP = cast<GetElementPtrInst>(B.Inst);
+
+    // If the instructions do not have the same inbounds restrictions, we do
+    // not consider them the same.
+    if (GEP->isInBounds() != OtherGEP->isInBounds())
+      return false;
+
+    auto ZippedOperands = zip(GEP->indices(), OtherGEP->indices());
+
+    auto ZIt = ZippedOperands.begin();
+
+    // We increment here since we do not care about the first instruction,
+    // we only care about the following operands since they must be the
+    // exact same to be considered similar.
+    return std::all_of(++ZIt, ZippedOperands.end(),
+                       [](std::tuple<llvm::Use &, llvm::Use &> R) {
+                         return std::get<0>(R) == std::get<1>(R);
+                       });
+  }
+
+  return true;
 }
 
 // TODO: This is the same as the MachineOutliner, and should be consolidated

llvm/test/Transforms/IROutliner/illegal-gep.ll

@@ -12,7 +12,8 @@ define void @function1(%struct.ST* %s, i64 %t) {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[A:%.*]] = alloca i32, align 4
 ; CHECK-NEXT:    [[B:%.*]] = alloca i32, align 4
-; CHECK-NEXT:    call void @outlined_ir_func_0(i32* [[A]], i32* [[B]])
+; CHECK-NEXT:    store i32 2, i32* [[A]], align 4
+; CHECK-NEXT:    store i32 3, i32* [[B]], align 4
 ; CHECK-NEXT:    [[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST:%.*]], %struct.ST* [[S:%.*]], i64 1
 ; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr inbounds [[STRUCT_ST]], %struct.ST* [[S]], i64 [[T:%.*]]
 ; CHECK-NEXT:    ret void
@@ -32,7 +33,8 @@ define void @function2(%struct.ST* %s, i64 %t) {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[A:%.*]] = alloca i32, align 4
 ; CHECK-NEXT:    [[B:%.*]] = alloca i32, align 4
-; CHECK-NEXT:    call void @outlined_ir_func_0(i32* [[A]], i32* [[B]])
+; CHECK-NEXT:    store i32 2, i32* [[A]], align 4
+; CHECK-NEXT:    store i32 3, i32* [[B]], align 4
 ; CHECK-NEXT:    [[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST:%.*]], %struct.ST* [[S:%.*]], i64 1
 ; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr inbounds [[STRUCT_ST]], %struct.ST* [[S]], i64 [[T:%.*]]
 ; CHECK-NEXT:    ret void

llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp

@@ -815,16 +815,17 @@ TEST(IRInstructionMapper, AllocaIllegal) {
   ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(0));
 }
 
-// Checks that an getelementptr instruction is mapped to be illegal.  There is
+// Checks that an getelementptr instruction is mapped to be legal.  And that
-// extra checking required for the parameters if a getelementptr has more than
+// the operands in getelementpointer instructions are the exact same after the
-// two operands.
+// first element operand, which only requires the same type.
-TEST(IRInstructionMapper, GetElementPtrIllegal) {
+TEST(IRInstructionMapper, GetElementPtrSameEndOperands) {
   StringRef ModuleString = R"(
     %struct.RT = type { i8, [10 x [20 x i32]], i8 }
     %struct.ST = type { i32, double, %struct.RT }
-    define i32 @f(%struct.ST* %s, i32 %a, i32 %b) {
+    define i32 @f(%struct.ST* %s, i64 %a, i64 %b) {
     bb0:
-       %0 = getelementptr inbounds %struct.ST, %struct.ST* %s, i64 1
+       %0 = getelementptr inbounds %struct.ST, %struct.ST* %s, i64 %a, i32 0
+       %1 = getelementptr inbounds %struct.ST, %struct.ST* %s, i64 %b, i32 0
        ret i32 0
     })";
   LLVMContext Context;
@@ -839,7 +840,93 @@ TEST(IRInstructionMapper, GetElementPtrIllegal) {
   getVectors(*M, Mapper, InstrList, UnsignedVec);
 
   ASSERT_EQ(InstrList.size(), UnsignedVec.size());
-  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(0));
+  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(3));
+  ASSERT_EQ(UnsignedVec[0], UnsignedVec[1]);
+}
+
+// Check that when the operands in getelementpointer instructions are not the
+// exact same after the first element operand, the instructions are mapped to
+// different values.
+TEST(IRInstructionMapper, GetElementPtrDifferentEndOperands) {
+  StringRef ModuleString = R"(
+    %struct.RT = type { i8, [10 x [20 x i32]], i8 }
+    %struct.ST = type { i32, double, %struct.RT }
+    define i32 @f(%struct.ST* %s, i64 %a, i64 %b) {
+    bb0:
+       %0 = getelementptr inbounds %struct.ST, %struct.ST* %s, i64 %a, i32 0
+       %1 = getelementptr inbounds %struct.ST, %struct.ST* %s, i64 %b, i32 2
+       ret i32 0
+    })";
+  LLVMContext Context;
+  std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);
+
+  std::vector<IRInstructionData *> InstrList;
+  std::vector<unsigned> UnsignedVec;
+
+  SpecificBumpPtrAllocator<IRInstructionData> InstDataAllocator;
+  SpecificBumpPtrAllocator<IRInstructionDataList> IDLAllocator;
+  IRInstructionMapper Mapper(&InstDataAllocator, &IDLAllocator);
+  getVectors(*M, Mapper, InstrList, UnsignedVec);
+
+  ASSERT_EQ(InstrList.size(), UnsignedVec.size());
+  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(3));
+  ASSERT_NE(UnsignedVec[0], UnsignedVec[1]);
+}
+
+// Check that when the operands in getelementpointer instructions are not the
+// same initial base type, each instruction is mapped to a different value.
+TEST(IRInstructionMapper, GetElementPtrDifferentBaseType) {
+  StringRef ModuleString = R"(
+    %struct.RT = type { i8, [10 x [20 x i32]], i8 }
+    %struct.ST = type { i32, double, %struct.RT }
+    define i32 @f(%struct.ST* %s, %struct.RT* %r, i64 %a, i64 %b) {
+    bb0:
+       %0 = getelementptr inbounds %struct.ST, %struct.ST* %s, i64 %a
+       %1 = getelementptr inbounds %struct.RT, %struct.RT* %r, i64 %b
+       ret i32 0
+    })";
+  LLVMContext Context;
+  std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);
+
+  std::vector<IRInstructionData *> InstrList;
+  std::vector<unsigned> UnsignedVec;
+
+  SpecificBumpPtrAllocator<IRInstructionData> InstDataAllocator;
+  SpecificBumpPtrAllocator<IRInstructionDataList> IDLAllocator;
+  IRInstructionMapper Mapper(&InstDataAllocator, &IDLAllocator);
+  getVectors(*M, Mapper, InstrList, UnsignedVec);
+
+  ASSERT_EQ(InstrList.size(), UnsignedVec.size());
+  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(3));
+  ASSERT_NE(UnsignedVec[0], UnsignedVec[1]);
+}
+
+// Check that when the operands in getelementpointer instructions do not have
+// the same inbounds modifier, they are not counted as the same.
+TEST(IRInstructionMapper, GetElementPtrDifferentInBounds) {
+  StringRef ModuleString = R"(
+    %struct.RT = type { i8, [10 x [20 x i32]], i8 }
+    %struct.ST = type { i32, double, %struct.RT }
+    define i32 @f(%struct.ST* %s, %struct.RT* %r, i64 %a, i64 %b) {
+    bb0:
+       %0 = getelementptr inbounds %struct.ST, %struct.ST* %s, i64 %a, i32 0
+       %1 = getelementptr %struct.ST, %struct.ST* %s, i64 %b, i32 0
+       ret i32 0
+    })";
+  LLVMContext Context;
+  std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);
+
+  std::vector<IRInstructionData *> InstrList;
+  std::vector<unsigned> UnsignedVec;
+
+  SpecificBumpPtrAllocator<IRInstructionData> InstDataAllocator;
+  SpecificBumpPtrAllocator<IRInstructionDataList> IDLAllocator;
+  IRInstructionMapper Mapper(&InstDataAllocator, &IDLAllocator);
+  getVectors(*M, Mapper, InstrList, UnsignedVec);
+
+  ASSERT_EQ(InstrList.size(), UnsignedVec.size());
+  ASSERT_EQ(UnsignedVec.size(), static_cast<unsigned>(3));
+  ASSERT_NE(UnsignedVec[0], UnsignedVec[1]);
 }
 
 // Checks that a call instruction is mapped to be illegal.  We have to perform