[llvm] r216693 - Don't promote byval pointer arguments when padding matters

Reid Kleckner reid at kleckner.net
Thu Aug 28 15:42:00 PDT 2014


Author: rnk
Date: Thu Aug 28 17:42:00 2014
New Revision: 216693

URL: http://llvm.org/viewvc/llvm-project?rev=216693&view=rev
Log:
Don't promote byval pointer arguments when padding matters

Don't promote byval pointer arguments when when their size in bits is
not equal to their alloc size in bits. This can happen for x86_fp80,
where the size in bits is 80 but the alloca size in bits in 128.
Promoting these types can break passing unions of x86_fp80s and other
types.

Patch by Thomas Jablin!

Reviewed By: rnk

Differential Revision: http://reviews.llvm.org/D5057

Added:
    llvm/trunk/test/Transforms/ArgumentPromotion/fp80.ll
Modified:
    llvm/trunk/lib/Transforms/IPO/ArgumentPromotion.cpp
    llvm/trunk/test/Transforms/ArgumentPromotion/tail.ll

Modified: llvm/trunk/lib/Transforms/IPO/ArgumentPromotion.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/ArgumentPromotion.cpp?rev=216693&r1=216692&r2=216693&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/ArgumentPromotion.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/ArgumentPromotion.cpp Thu Aug 28 17:42:00 2014
@@ -78,6 +78,8 @@ namespace {
 
     const DataLayout *DL;
   private:
+    bool isDenselyPacked(Type *type);
+    bool canPaddingBeAccessed(Argument *Arg);
     CallGraphNode *PromoteArguments(CallGraphNode *CGN);
     bool isSafeToPromoteArgument(Argument *Arg, bool isByVal) const;
     CallGraphNode *DoPromotion(Function *F,
@@ -125,6 +127,78 @@ bool ArgPromotion::runOnSCC(CallGraphSCC
   return Changed;
 }
 
+/// \brief Checks if a type could have padding bytes.
+bool ArgPromotion::isDenselyPacked(Type *type) {
+
+  // There is no size information, so be conservative.
+  if (!type->isSized())
+    return false;
+
+  // If the alloc size is not equal to the storage size, then there are padding
+  // bytes. For x86_fp80 on x86-64, size: 80 alloc size: 128.
+  if (!DL || DL->getTypeSizeInBits(type) != DL->getTypeAllocSizeInBits(type))
+    return false;
+
+  if (!isa<CompositeType>(type))
+    return true;
+
+  // For homogenous sequential types, check for padding within members.
+  if (SequentialType *seqTy = dyn_cast<SequentialType>(type))
+    return isa<PointerType>(seqTy) || isDenselyPacked(seqTy->getElementType());
+
+  // Check for padding within and between elements of a struct.
+  StructType *StructTy = cast<StructType>(type);
+  const StructLayout *Layout = DL->getStructLayout(StructTy);
+  uint64_t StartPos = 0;
+  for (unsigned i = 0, E = StructTy->getNumElements(); i < E; ++i) {
+    Type *ElTy = StructTy->getElementType(i);
+    if (!isDenselyPacked(ElTy))
+      return false;
+    if (StartPos != Layout->getElementOffsetInBits(i))
+      return false;
+    StartPos += DL->getTypeAllocSizeInBits(ElTy);
+  }
+
+  return true;
+}
+
+/// \brief Checks if the padding bytes of an argument could be accessed.
+bool ArgPromotion::canPaddingBeAccessed(Argument *arg) {
+
+  assert(arg->hasByValAttr());
+
+  // Track all the pointers to the argument to make sure they are not captured.
+  SmallPtrSet<Value *, 16> PtrValues;
+  PtrValues.insert(arg);
+
+  // Track all of the stores.
+  SmallVector<StoreInst *, 16> Stores;
+
+  // Scan through the uses recursively to make sure the pointer is always used
+  // sanely.
+  SmallVector<Value *, 16> WorkList;
+  WorkList.insert(WorkList.end(), arg->user_begin(), arg->user_end());
+  while (!WorkList.empty()) {
+    Value *V = WorkList.back();
+    WorkList.pop_back();
+    if (isa<GetElementPtrInst>(V) || isa<PHINode>(V)) {
+      if (PtrValues.insert(V))
+        WorkList.insert(WorkList.end(), V->user_begin(), V->user_end());
+    } else if (StoreInst *Store = dyn_cast<StoreInst>(V)) {
+      Stores.push_back(Store);
+    } else if (!isa<LoadInst>(V)) {
+      return true;
+    }
+  }
+
+// Check to make sure the pointers aren't captured
+  for (StoreInst *Store : Stores)
+    if (PtrValues.count(Store->getValueOperand()))
+      return true;
+
+  return false;
+}
+
 /// PromoteArguments - This method checks the specified function to see if there
 /// are any promotable arguments and if it is safe to promote the function (for
 /// example, all callers are direct).  If safe to promote some arguments, it
@@ -172,9 +246,13 @@ CallGraphNode *ArgPromotion::PromoteArgu
     Type *AgTy = cast<PointerType>(PtrArg->getType())->getElementType();
 
     // If this is a byval argument, and if the aggregate type is small, just
-    // pass the elements, which is always safe.  This does not apply to
-    // inalloca.
-    if (PtrArg->hasByValAttr()) {
+    // pass the elements, which is always safe, if the passed value is densely
+    // packed or if we can prove the padding bytes are never accessed. This does
+    // not apply to inalloca.
+    bool isSafeToPromote =
+      PtrArg->hasByValAttr() &&
+      (isDenselyPacked(AgTy) || !canPaddingBeAccessed(PtrArg));
+    if (isSafeToPromote) {
       if (StructType *STy = dyn_cast<StructType>(AgTy)) {
         if (maxElements > 0 && STy->getNumElements() > maxElements) {
           DEBUG(dbgs() << "argpromotion disable promoting argument '"

Added: llvm/trunk/test/Transforms/ArgumentPromotion/fp80.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ArgumentPromotion/fp80.ll?rev=216693&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/ArgumentPromotion/fp80.ll (added)
+++ llvm/trunk/test/Transforms/ArgumentPromotion/fp80.ll Thu Aug 28 17:42:00 2014
@@ -0,0 +1,58 @@
+; RUN: opt < %s -argpromotion -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%union.u = type { x86_fp80 }
+%struct.s = type { double, i16, i8, [5 x i8] }
+
+ at b = internal global %struct.s { double 3.14, i16 9439, i8 25, [5 x i8] undef }, align 16
+
+%struct.Foo = type { i32, i64 }
+ at a = internal global %struct.Foo { i32 1, i64 2 }, align 8
+
+define void @run() {
+entry:
+  tail call i8 @UseLongDoubleUnsafely(%union.u* byval align 16 bitcast (%struct.s* @b to %union.u*))
+  tail call x86_fp80 @UseLongDoubleSafely(%union.u* byval align 16 bitcast (%struct.s* @b to %union.u*))
+  call i64 @AccessPaddingOfStruct(%struct.Foo* @a)
+  call i64 @CaptureAStruct(%struct.Foo* @a)
+  ret void
+}
+
+; CHECK: internal i8 @UseLongDoubleUnsafely(%union.u* byval align 16 %arg) {
+define internal i8 @UseLongDoubleUnsafely(%union.u* byval align 16 %arg) {
+entry:
+  %bitcast = bitcast %union.u* %arg to %struct.s*
+  %gep = getelementptr inbounds %struct.s* %bitcast, i64 0, i32 2
+  %result = load i8* %gep
+  ret i8 %result
+}
+
+; CHECK: internal x86_fp80 @UseLongDoubleSafely(x86_fp80 {{%.*}}) {
+define internal x86_fp80 @UseLongDoubleSafely(%union.u* byval align 16 %arg) {
+  %gep = getelementptr inbounds %union.u* %arg, i64 0, i32 0
+  %fp80 = load x86_fp80* %gep
+  ret x86_fp80 %fp80
+}
+
+; CHECK: define internal i64 @AccessPaddingOfStruct(%struct.Foo* byval %a) {
+define internal i64 @AccessPaddingOfStruct(%struct.Foo* byval %a) {
+  %p = bitcast %struct.Foo* %a to i64*
+  %v = load i64* %p
+  ret i64 %v
+}
+
+; CHECK: define internal i64 @CaptureAStruct(%struct.Foo* byval %a) {
+define internal i64 @CaptureAStruct(%struct.Foo* byval %a) {
+entry:
+  %a_ptr = alloca %struct.Foo*
+  br label %loop
+
+loop:
+  %phi = phi %struct.Foo* [ null, %entry ], [ %gep, %loop ]
+  %0   = phi %struct.Foo* [ %a, %entry ],   [ %0, %loop ]
+  store %struct.Foo* %phi, %struct.Foo** %a_ptr
+  %gep = getelementptr %struct.Foo* %a, i64 0
+  br label %loop
+}

Modified: llvm/trunk/test/Transforms/ArgumentPromotion/tail.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ArgumentPromotion/tail.ll?rev=216693&r1=216692&r2=216693&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/ArgumentPromotion/tail.ll (original)
+++ llvm/trunk/test/Transforms/ArgumentPromotion/tail.ll Thu Aug 28 17:42:00 2014
@@ -1,6 +1,8 @@
 ; RUN: opt %s -argpromotion -S -o - | FileCheck %s
 ; PR14710
 
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
 %pair = type { i32, i32 }
 
 declare i8* @foo(%pair*)





More information about the llvm-commits mailing list