[llvm-branch-commits] [llvm] d974ac0 - [IRSim] Letting gep instructions be legal for similarity identification.

Andrew Litteken via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Thu Dec 31 12:46:44 PST 2020


Author: Andrew Litteken
Date: 2020-12-31T14:41:14-06:00
New Revision: d974ac0224dec34b95fb1be8c61bd8b524698bcd

URL: https://github.com/llvm/llvm-project/commit/d974ac0224dec34b95fb1be8c61bd8b524698bcd
DIFF: https://github.com/llvm/llvm-project/commit/d974ac0224dec34b95fb1be8c61bd8b524698bcd.diff

LOG: [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.

Added: 
    

Modified: 
    llvm/include/llvm/Analysis/IRSimilarityIdentifier.h
    llvm/lib/Analysis/IRSimilarityIdentifier.cpp
    llvm/test/Transforms/IROutliner/illegal-gep.ll
    llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Analysis/IRSimilarityIdentifier.h b/llvm/include/llvm/Analysis/IRSimilarityIdentifier.h
index 99a5fcb3a578..a3004ca0dc59 100644
--- a/llvm/include/llvm/Analysis/IRSimilarityIdentifier.h
+++ b/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.

diff  --git a/llvm/lib/Analysis/IRSimilarityIdentifier.cpp b/llvm/lib/Analysis/IRSimilarityIdentifier.cpp
index 141c1e0a5d03..c276e3f28d73 100644
--- a/llvm/lib/Analysis/IRSimilarityIdentifier.cpp
+++ b/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))
-    return true;
+  if (!A.Inst->isSameOperationAs(B.Inst)) {
+    // If there is a predicate, this means that either there is a swapped
+    // predicate, or that the types are 
diff erent, we want to make sure that
+    // the predicates are equivalent via swapping.
+    if (isa<CmpInst>(A.Inst) && isa<CmpInst>(B.Inst)) {
+
+      if (A.getPredicate() != B.getPredicate())
+        return false;
+
+      // If the predicates are the same via swap, make sure that the types are
+      // still the same.
+      auto ZippedTypes = zip(A.OperVals, B.OperVals);
+
+      return all_of(
+          ZippedTypes, [](std::tuple<llvm::Value *, llvm::Value *> R) {
+            return std::get<0>(R)->getType() == std::get<1>(R)->getType();
+          });
+    }
 
-  // If there is a predicate, this means that either there is a swapped
-  // predicate, or that the types are 
diff erent, we want to make sure that
-  // the predicates are equivalent via swapping.
-  if (isa<CmpInst>(A.Inst) && isa<CmpInst>(B.Inst)) {
+    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 (A.getPredicate() != B.getPredicate())
+    // If the instructions do not have the same inbounds restrictions, we do
+    // not consider them the same.
+    if (GEP->isInBounds() != OtherGEP->isInBounds())
       return false;
 
-    // If the predicates are the same via swap, make sure that the types are
-    // still the same.
-    auto ZippedTypes = zip(A.OperVals, B.OperVals);
+    auto ZippedOperands = zip(GEP->indices(), OtherGEP->indices());
 
-    return all_of(ZippedTypes, [](std::tuple<llvm::Value *, llvm::Value *> R) {
-      return std::get<0>(R)->getType() == std::get<1>(R)->getType();
-    });
+    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 false;
+  return true;
 }
 
 // TODO: This is the same as the MachineOutliner, and should be consolidated

diff  --git a/llvm/test/Transforms/IROutliner/illegal-gep.ll b/llvm/test/Transforms/IROutliner/illegal-gep.ll
index a625106105b0..5f009617c4b3 100644
--- a/llvm/test/Transforms/IROutliner/illegal-gep.ll
+++ b/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

diff  --git a/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp b/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp
index a28847b47126..ba33144c8ef6 100644
--- a/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp
+++ b/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
-// extra checking required for the parameters if a getelementptr has more than
-// two operands.
-TEST(IRInstructionMapper, GetElementPtrIllegal) {
+// Checks that an getelementptr instruction is mapped to be legal.  And that
+// the operands in getelementpointer instructions are the exact same after the
+// first element operand, which only requires the same type.
+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
+// 
diff erent 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 
diff erent 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


        


More information about the llvm-branch-commits mailing list