[llvm-commits] [poolalloc] r164795 - in /poolalloc/trunk: lib/DSA/Local.cpp test/dsa/regression/2012-09-27.ConstantAggregate.ll

[Will Dietz]

Author: wdietz2
Date: Thu Sep 27 16:10:59 2012
New Revision: 164795

URL: http://llvm.org/viewvc/llvm-project?rev=164795&view=rev
Log:
Local: Much better handling of InsertValue/ExtractValue instructions.

* Don't abort if a constant aggregate is used as an operand.
  Instead, process them the same way we handle global initializers.
  This improvement applies to all users of getValueDest, although
  I don't think getValueDest is called on non-pointer types elsewhere.
* Support use of these instructions to index into arrays, structs,
  and combinations of both.
* Add regression test checking we at least process these instructions,
  and check that track pointers inserted/extracted from aggregates
  in at least a simple case.

Thanks to Joshua for reporting the original issue!
  (assertion failure on insertvalue of constant struct)

Added:
    poolalloc/trunk/test/dsa/regression/2012-09-27.ConstantAggregate.ll
Modified:
    poolalloc/trunk/lib/DSA/Local.cpp

Modified: poolalloc/trunk/lib/DSA/Local.cpp
URL: http://llvm.org/viewvc/llvm-project/poolalloc/trunk/lib/DSA/Local.cpp?rev=164795&r1=164794&r2=164795&view=diff
==============================================================================
--- poolalloc/trunk/lib/DSA/Local.cpp (original)
+++ poolalloc/trunk/lib/DSA/Local.cpp Thu Sep 27 16:10:59 2012
@@ -303,6 +303,13 @@
       //
       N = createNode();
       N->setUnknownMarker();
+    } else if (isa<ConstantStruct>(C) || isa<ConstantArray>(C) ||
+               isa<ConstantDataSequential>(C) || isa<ConstantDataArray>(C) ||
+               isa<ConstantDataVector>(C)) {
+      // Treat these the same way we treat global initializers
+      N = createNode();
+      NH.mergeWith(N);
+      MergeConstantInitIntoNode(NH, C->getType(), C);
     } else {
       errs() << "Unknown constant: " << *C << "\n";
       assert(0 && "Unknown constant type!");
@@ -604,6 +611,36 @@
   //Address can escape through cmps
 }
 
+unsigned getValueOffset(Type *Ty, ArrayRef<unsigned> Idxs,
+                        const TargetData &TD) {
+  unsigned Offset = 0;
+  for (ArrayRef<unsigned>::iterator I = Idxs.begin(), E = Idxs.end(); I != E;
+       ++I) {
+    // Lifted from TargetData.cpp's getIndexedOffset.
+    // We can't use that because it insists on only allowing pointer types.
+    if (StructType *STy = dyn_cast<StructType>(Ty)) {
+      unsigned FieldNo = *I;
+
+      // Get structure layout information...
+      const StructLayout *Layout = TD.getStructLayout(STy);
+
+      // Add in the offset, as calculated by the structure layout info...
+      Offset += Layout->getElementOffset(FieldNo);
+
+      // Update Ty to refer to current element
+      Ty = STy->getElementType(FieldNo);
+    } else {
+      // Update Ty to refer to current element
+      Ty = cast<SequentialType>(Ty)->getElementType();
+
+      // Get the array index and the size of each array element.
+      int64_t arrayIdx = *I;
+      Offset += (uint64_t)arrayIdx * TD.getTypeAllocSize(Ty);
+    }
+  }
+  return Offset;
+}
+
 void GraphBuilder::visitInsertValueInst(InsertValueInst& I) {
   setDestTo(I, createNode()->setAllocaMarker());
 
@@ -613,17 +650,12 @@
 
   // Mark that the node is written to...
   Dest.getNode()->setModifiedMarker();
-  unsigned Offset = 0;
   Type* STy = I.getAggregateOperand()->getType();
-  llvm::InsertValueInst::idx_iterator i = I.idx_begin(), e = I.idx_end(); 
-  for (; i != e; i++) {
-    const StructLayout *SL = TD.getStructLayout(cast<StructType>(STy));
-    Offset += SL->getElementOffset(*i);
-    STy = (cast<StructType>(STy))->getTypeAtIndex(*i);
-  }
+
+  unsigned Offset = getValueOffset(STy, I.getIndices(), TD);
 
   // Ensure a type-record exists...
-  Dest.getNode()->mergeTypeInfo(StoredTy, Offset); 
+  Dest.getNode()->mergeTypeInfo(StoredTy, Offset);
 
   // Avoid adding edges from null, or processing non-"pointer" stores
   if (isa<PointerType>(StoredTy))
@@ -631,18 +663,13 @@
 }
 
 void GraphBuilder::visitExtractValueInst(ExtractValueInst& I) {
-  DSNodeHandle Ptr = getValueDest(I.getOperand(0));
+  DSNodeHandle Ptr = getValueDest(I.getAggregateOperand());
 
   // Make that the node is read from...
   Ptr.getNode()->setReadMarker();
-  unsigned Offset = 0;
   Type* STy = I.getAggregateOperand()->getType();
-  llvm::ExtractValueInst::idx_iterator i = I.idx_begin(), e = I.idx_end();
-  for (; i != e; i++) {
-    const StructLayout *SL = TD.getStructLayout(cast<StructType>(STy));
-    Offset += SL->getElementOffset(*i);
-    STy = (cast<StructType>(STy))->getTypeAtIndex(*i);
-  }
+
+  unsigned Offset = getValueOffset(STy, I.getIndices(), TD);
 
   // Ensure a typerecord exists...
   Ptr.getNode()->mergeTypeInfo(I.getType(), Offset);

Added: poolalloc/trunk/test/dsa/regression/2012-09-27.ConstantAggregate.ll
URL: http://llvm.org/viewvc/llvm-project/poolalloc/trunk/test/dsa/regression/2012-09-27.ConstantAggregate.ll?rev=164795&view=auto
==============================================================================
--- poolalloc/trunk/test/dsa/regression/2012-09-27.ConstantAggregate.ll (added)
+++ poolalloc/trunk/test/dsa/regression/2012-09-27.ConstantAggregate.ll Thu Sep 27 16:10:59 2012
@@ -0,0 +1,31 @@
+;RUN: dsaopt %s -dsa-eq -disable-output
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @test() nounwind {
+entry:
+  %ptr = alloca i32
+  %ptr2 = alloca i32
+  ; Handle insertvalue on constantstruct
+  %struct = insertvalue {i64, i32*} { i64 10, i32* null }, i32 *%ptr, 1
+  %struct2 = insertvalue {i64, i32*} %struct, i32 *%ptr2, 1
+  ; Load the pointer (unification says may be ptr or ptr2),
+  ; and verify we tracked it properly:
+;RUN: dsaopt %s -dsa-local -analyze -check-same-node=test:ptr,test:ptr2,test:both
+  %both = extractvalue {i64, i32*} %struct, 1
+  ; Ensure handle extractvalue on constantstruct also
+  %val = extractvalue {i64, i64} { i64 10, i64 5 }, 1
+
+  ret void
+}
+
+; While we're at it, verify we don't die on similar constructs
+; when accessing (constant) arrays/vectors.
+define void @ArrayAndVector() nounwind {
+entry:
+  %0 = extractvalue [2 x i64] [i64 10, i64 20], 1
+  %1 = insertvalue [2 x i64] [i64 10, i64 20], i64 %0, 1
+  %2 = extractelement <2 x i64> <i64 30, i64 40>, i32 1
+  %3 = insertelement <2 x i64> <i64 30, i64 40>, i64 %2, i32 1
+  ret void
+}