[llvm] 8269470 - [llvm-diff] Explicitly check ConstantStructs for differences

Bill Wendling via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 23 16:26:46 PDT 2021


Author: Bill Wendling
Date: 2021-06-23T16:26:34-07:00
New Revision: 826947080b7e884c4758eff6e6646f7df4cc5380

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

LOG: [llvm-diff] Explicitly check ConstantStructs for differences

A ConstantStruct is renamed when the LLVM context sees a new one. This
makes global variable initializers appear different when they aren't.
Instead, check the ConstantStruct for equivalence.

Differential Revision: https://reviews.llvm.org/D104734

Added: 
    

Modified: 
    llvm/test/tools/llvm-diff/initializers.ll
    llvm/tools/llvm-diff/DifferenceEngine.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/tools/llvm-
diff /initializers.ll b/llvm/test/tools/llvm-
diff /initializers.ll
index 7f59e65e8b40..93cabfba54f2 100644
--- a/llvm/test/tools/llvm-
diff /initializers.ll
+++ b/llvm/test/tools/llvm-
diff /initializers.ll
@@ -10,3 +10,29 @@ define void @foo() {
   %1 = getelementptr [2 x i16*], [2 x i16*]* @gv2, i64 0, i64 undef
   ret void
 }
+
+; A named structure may be renamed when the right module is read. This is due
+; to the LLParser being 
diff erent between the left and right modules, and the
+; context renaming one.
+
+%struct.ty1 = type { i16, i16 }
+
+ at gv3 = internal global [1 x %struct.ty1] [%struct.ty1 { i16 928, i16 0 }], align 16
+
+define void @bar() {
+  %1 = getelementptr [1 x %struct.ty1], [1 x %struct.ty1]* @gv3, i64 0, i64 undef
+  ret void
+}
+
+; An initializer may reference the variable it's initializing via bitcast /
+; GEP. Check that it doesn't cause an infinite loop.
+
+%struct.mutex = type { %struct.list_head }
+%struct.list_head = type { %struct.list_head*, %struct.list_head* }
+
+ at vmx_l1d_flush_mutex = internal global %struct.mutex { %struct.list_head { %struct.list_head* bitcast (i8* getelementptr (i8, i8* bitcast (%struct.mutex* @vmx_l1d_flush_mutex to i8*), i64 16) to %struct.list_head*), %struct.list_head* bitcast (i8* getelementptr (i8, i8* bitcast (%struct.mutex* @vmx_l1d_flush_mutex to i8*), i64 16) to %struct.list_head*) } }, align 8
+
+define internal i32 @qux() {
+  call void undef(%struct.mutex* @vmx_l1d_flush_mutex)
+  ret i32 undef
+}

diff  --git a/llvm/tools/llvm-
diff /DifferenceEngine.cpp b/llvm/tools/llvm-
diff /DifferenceEngine.cpp
index cc9dce04de05..b3a6ba83c1e4 100644
--- a/llvm/tools/llvm-
diff /DifferenceEngine.cpp
+++ b/llvm/tools/llvm-
diff /DifferenceEngine.cpp
@@ -113,6 +113,12 @@ class PriorityQueue {
 class FunctionDifferenceEngine {
   DifferenceEngine &Engine;
 
+  // Some initializers may reference the variable we're currently checking. This
+  // can cause an infinite loop. The Saved[LR]HS ivars can be checked to prevent
+  // recursing.
+  const Value *SavedLHS;
+  const Value *SavedRHS;
+
   /// The current mapping from old local values to new local values.
   DenseMap<const Value *, const Value *> Values;
 
@@ -412,7 +418,7 @@ class FunctionDifferenceEngine {
 
     if (L->getValueID() != R->getValueID())
       return false;
-    
+
     // Ask the engine about global values.
     if (isa<GlobalValue>(L))
       return Engine.equivalentAsOperands(cast<GlobalValue>(L),
@@ -465,12 +471,40 @@ class FunctionDifferenceEngine {
       return true;
     }
 
+    // If L and R are ConstantStructs, compare each field and type.
+    if (isa<ConstantStruct>(L)) {
+      const ConstantStruct *CSL = cast<ConstantStruct>(L);
+      const ConstantStruct *CSR = cast<ConstantStruct>(R);
+
+      const StructType *LTy = cast<StructType>(CSL->getType());
+      const StructType *RTy = cast<StructType>(CSR->getType());
+
+      // The StructTypes should have the same attributes. Don't use
+      // isLayoutIdentical(), because that just checks the element pointers,
+      // which may not work here.
+      if (LTy->getNumElements() != RTy->getNumElements() ||
+          LTy->isPacked() != RTy->isPacked())
+        return false;
+
+      for (unsigned I = 0; I < LTy->getNumElements(); I++) {
+        const Value *LAgg = CSL->getAggregateElement(I);
+        const Value *RAgg = CSR->getAggregateElement(I);
+
+        if (!equivalentAsOperands(LAgg, RAgg)) {
+          return false;
+        }
+      }
+
+      return true;
+    }
+
     return false;
   }
 
   bool equivalentAsOperands(const ConstantExpr *L, const ConstantExpr *R) {
     if (L == R)
       return true;
+
     if (L->getOpcode() != R->getOpcode())
       return false;
 
@@ -492,9 +526,23 @@ class FunctionDifferenceEngine {
     if (L->getNumOperands() != R->getNumOperands())
       return false;
 
-    for (unsigned I = 0, E = L->getNumOperands(); I != E; ++I)
-      if (!equivalentAsOperands(L->getOperand(I), R->getOperand(I)))
+    for (unsigned I = 0, E = L->getNumOperands(); I != E; ++I) {
+      const auto *LOp = L->getOperand(I);
+      const auto *ROp = R->getOperand(I);
+
+      if (LOp == SavedLHS || ROp == SavedRHS) {
+        if (LOp != SavedLHS || ROp != SavedRHS)
+          // If the left and right operands aren't both re-analyzing the
+          // variable, then the initialiers don't match, so report "false".
+          // Otherwise, we skip these operands..
+          return false;
+
+        continue;
+      }
+
+      if (!equivalentAsOperands(LOp, ROp))
         return false;
+    }
 
     return true;
   }
@@ -528,8 +576,11 @@ class FunctionDifferenceEngine {
   FunctionDifferenceEngine *this_() { return this; }
 
 public:
-  FunctionDifferenceEngine(DifferenceEngine &Engine) :
-    Engine(Engine), Queue(QueueSorter(*this_())) {}
+  FunctionDifferenceEngine(DifferenceEngine &Engine,
+                           const Value *SavedLHS = nullptr,
+                           const Value *SavedRHS = nullptr)
+      : Engine(Engine), SavedLHS(SavedLHS), SavedRHS(SavedRHS),
+        Queue(QueueSorter(*this_())) {}
 
   void 
diff (const Function *L, const Function *R) {
     if (L->arg_size() != R->arg_size())
@@ -785,8 +836,8 @@ bool DifferenceEngine::equivalentAsOperands(const GlobalValue *L,
     const GlobalVariable *GVR = cast<GlobalVariable>(R);
     if (GVL->hasLocalLinkage() && GVL->hasUniqueInitializer() &&
         GVR->hasLocalLinkage() && GVR->hasUniqueInitializer())
-      return FunctionDifferenceEngine(*this).equivalentAsOperands(
-          GVL->getInitializer(), GVR->getInitializer());
+      return FunctionDifferenceEngine(*this, GVL, GVR)
+          .equivalentAsOperands(GVL->getInitializer(), GVR->getInitializer());
   }
 
   return L->getName() == R->getName();


        


More information about the llvm-commits mailing list