[llvm-commits] [llvm] r84199 - in /llvm/trunk: include/llvm/Analysis/MallocHelper.h lib/Analysis/MallocHelper.cpp lib/Analysis/PointerTracking.cpp lib/Transforms/IPO/GlobalOpt.cpp

Victor Hernandez vhernandez at apple.com
Thu Oct 15 13:14:53 PDT 2009


Author: hernande
Date: Thu Oct 15 15:14:52 2009
New Revision: 84199

URL: http://llvm.org/viewvc/llvm-project?rev=84199&view=rev
Log:
Fix bug where array malloc with unexpected computation of the size argument resulted in MallocHelper 
identifying the malloc as a non-array malloc.  This broke GlobalOpt's optimization of stores of mallocs 
to global variables.

The fix is to classify malloc's into 3 categories:
1. non-array mallocs
2. array mallocs whose array size can be determined
3. mallocs that cannot be determined to be of type 1 or 2 and cannot be optimized

getMallocArraySize() returns NULL for category 3, and all users of this function must avoid their 
malloc optimization if this function returns NULL.

Eventually, currently unexpected codegen for computing the malloc's size argument will be supported in
isArrayMalloc() and getMallocArraySize(), extending malloc optimizations to those examples.


Modified:
    llvm/trunk/include/llvm/Analysis/MallocHelper.h
    llvm/trunk/lib/Analysis/MallocHelper.cpp
    llvm/trunk/lib/Analysis/PointerTracking.cpp
    llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp

Modified: llvm/trunk/include/llvm/Analysis/MallocHelper.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/MallocHelper.h?rev=84199&r1=84198&r2=84199&view=diff

==============================================================================
--- llvm/trunk/include/llvm/Analysis/MallocHelper.h (original)
+++ llvm/trunk/include/llvm/Analysis/MallocHelper.h Thu Oct 15 15:14:52 2009
@@ -46,9 +46,9 @@
 /// matches the malloc call IR generated by CallInst::CreateMalloc().  This 
 /// means that it is a malloc call with one bitcast use AND the malloc call's 
 /// size argument is:
-///  1. a constant not equal to the malloc's allocated type
+///  1. a constant not equal to the size of the malloced type
 /// or
-///  2. the result of a multiplication by the malloc's allocated type
+///  2. the result of a multiplication by the size of the malloced type
 /// Otherwise it returns NULL.
 /// The unique bitcast is needed to determine the type/size of the array
 /// allocation.
@@ -66,18 +66,17 @@
 /// unique bitcast use, then return NULL.
 const Type* getMallocAllocatedType(const CallInst* CI);
 
-/// getMallocArraySize - Returns the array size of a malloc call.  The array
-/// size is computated in 1 of 3 ways:
-///  1. If the element type if of size 1, then array size is the argument to 
+/// getMallocArraySize - Returns the array size of a malloc call.  For array
+/// mallocs, the size is computated in 1 of 3 ways:
+///  1. If the element type is of size 1, then array size is the argument to 
 ///     malloc.
 ///  2. Else if the malloc's argument is a constant, the array size is that
 ///     argument divided by the element type's size.
 ///  3. Else the malloc argument must be a multiplication and the array size is
 ///     the first operand of the multiplication.
-/// This function returns constant 1 if:
-///  1. The malloc call's allocated type cannot be determined.
-///  2. IR wasn't created by a call to CallInst::CreateMalloc() with a non-NULL
-///     ArraySize.
+/// For non-array mallocs, the computed size is constant 1. 
+/// This function returns NULL for all mallocs whose array size cannot be
+/// determined.
 Value* getMallocArraySize(CallInst* CI, LLVMContext &Context,
                           const TargetData* TD);
 

Modified: llvm/trunk/lib/Analysis/MallocHelper.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MallocHelper.cpp?rev=84199&r1=84198&r2=84199&view=diff

==============================================================================
--- llvm/trunk/lib/Analysis/MallocHelper.cpp (original)
+++ llvm/trunk/lib/Analysis/MallocHelper.cpp Thu Oct 15 15:14:52 2009
@@ -130,9 +130,9 @@
 /// matches the malloc call IR generated by CallInst::CreateMalloc().  This 
 /// means that it is a malloc call with one bitcast use AND the malloc call's 
 /// size argument is:
-///  1. a constant not equal to the malloc's allocated type
+///  1. a constant not equal to the size of the malloced type
 /// or
-///  2. the result of a multiplication by the malloc's allocated type
+///  2. the result of a multiplication by the size of the malloced type
 /// Otherwise it returns NULL.
 /// The unique bitcast is needed to determine the type/size of the array
 /// allocation.
@@ -183,25 +183,60 @@
   return PT ? PT->getElementType() : NULL;
 }
 
+/// isSafeToGetMallocArraySize - Returns true if the array size of a malloc can
+/// be determined.  It can be determined in these 3 cases of malloc codegen:
+/// 1. non-array malloc: The malloc's size argument is a constant and equals the ///    size of the type being malloced.
+/// 2. array malloc: This is a malloc call with one bitcast use AND the malloc
+///    call's size argument is a constant multiple of the size of the malloced
+///    type.
+/// 3. array malloc: This is a malloc call with one bitcast use AND the malloc
+///    call's size argument is the result of a multiplication by the size of the
+///    malloced type.
+/// Otherwise returns false.
+static bool isSafeToGetMallocArraySize(const CallInst *CI,
+                                       LLVMContext &Context,
+                                       const TargetData* TD) {
+  if (!CI)
+    return false;
+
+  // Type must be known to determine array size.
+  const Type* T = getMallocAllocatedType(CI);
+  if (!T) return false;
+
+  Value* MallocArg = CI->getOperand(1);
+  Constant *ElementSize = ConstantExpr::getSizeOf(T);
+  ElementSize = ConstantExpr::getTruncOrBitCast(ElementSize, 
+                                                MallocArg->getType());
+
+  // First, check if it is a non-array malloc.
+  if (isa<ConstantExpr>(MallocArg) && (MallocArg == ElementSize))
+    return true;
+
+  // Second, check if it can be determined that this is an array malloc.
+  return isArrayMallocHelper(CI, Context, TD);
+}
+
 /// isConstantOne - Return true only if val is constant int 1.
 static bool isConstantOne(Value *val) {
   return isa<ConstantInt>(val) && cast<ConstantInt>(val)->isOne();
 }
 
-/// getMallocArraySize - Returns the array size of a malloc call.  The array
-/// size is computated in 1 of 3 ways:
-///  1. If the element type if of size 1, then array size is the argument to 
+/// getMallocArraySize - Returns the array size of a malloc call.  For array
+/// mallocs, the size is computated in 1 of 3 ways:
+///  1. If the element type is of size 1, then array size is the argument to 
 ///     malloc.
 ///  2. Else if the malloc's argument is a constant, the array size is that
 ///     argument divided by the element type's size.
 ///  3. Else the malloc argument must be a multiplication and the array size is
 ///     the first operand of the multiplication.
-/// This function returns constant 1 if:
-///  1. The malloc call's allocated type cannot be determined.
-///  2. IR wasn't created by a call to CallInst::CreateMalloc() with a non-NULL
-///     ArraySize.
+/// For non-array mallocs, the computed size is constant 1. 
+/// This function returns NULL for all mallocs whose array size cannot be
+/// determined.
 Value* llvm::getMallocArraySize(CallInst* CI, LLVMContext &Context,
                                 const TargetData* TD) {
+  if (isSafeToGetMallocArraySize(CI, Context, TD))
+    return NULL;
+
   // Match CreateMalloc's use of constant 1 array-size for non-array mallocs.
   if (!isArrayMalloc(CI, Context, TD))
     return ConstantInt::get(CI->getOperand(1)->getType(), 1);

Modified: llvm/trunk/lib/Analysis/PointerTracking.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/PointerTracking.cpp?rev=84199&r1=84198&r2=84199&view=diff

==============================================================================
--- llvm/trunk/lib/Analysis/PointerTracking.cpp (original)
+++ llvm/trunk/lib/Analysis/PointerTracking.cpp Thu Oct 15 15:14:52 2009
@@ -102,8 +102,9 @@
 
   if (CallInst *CI = extractMallocCall(V)) {
     Value *arraySize = getMallocArraySize(CI, P->getContext(), TD);
-    Ty = getMallocAllocatedType(CI);
-    if (!Ty || !arraySize) return SE->getCouldNotCompute();
+    const Type* AllocTy = getMallocAllocatedType(CI);
+    if (!AllocTy || !arraySize) return SE->getCouldNotCompute();
+    Ty = AllocTy;
     // arraySize elements of type Ty.
     return SE->getSCEV(arraySize);
   }

Modified: llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp?rev=84199&r1=84198&r2=84199&view=diff

==============================================================================
--- llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp Thu Oct 15 15:14:52 2009
@@ -950,12 +950,14 @@
                                                      BitCastInst *BCI,
                                                      LLVMContext &Context,
                                                      TargetData* TD) {
+  DEBUG(errs() << "PROMOTING MALLOC GLOBAL: " << *GV
+               << "  CALL = " << *CI << "  BCI = " << *BCI << '\n');
+
   const Type *IntPtrTy = TD->getIntPtrType(Context);
   
-  DEBUG(errs() << "PROMOTING MALLOC GLOBAL: " << *GV << "  MALLOC = " << *CI);
-
-  ConstantInt *NElements = cast<ConstantInt>(getMallocArraySize(CI,
-                                                                Context, TD));
+  Value* ArraySize = getMallocArraySize(CI, Context, TD);
+  assert(ArraySize && "not a malloc whose array size can be determined");
+  ConstantInt *NElements = cast<ConstantInt>(ArraySize);
   if (NElements->getZExtValue() != 1) {
     // If we have an array allocation, transform it to a single element
     // allocation to make the code below simpler.
@@ -976,9 +978,6 @@
 
   // Create the new global variable.  The contents of the malloc'd memory is
   // undefined, so initialize with an undef value.
-  // FIXME: This new global should have the alignment returned by malloc.  Code
-  // could depend on malloc returning large alignment (on the mac, 16 bytes) but
-  // this would only guarantee some lower alignment.
   const Type *MAT = getMallocAllocatedType(CI);
   Constant *Init = UndefValue::get(MAT);
   GlobalVariable *NewGV = new GlobalVariable(*GV->getParent(), 
@@ -1892,16 +1891,17 @@
   // transform the program to use global memory instead of malloc'd memory.
   // This eliminates dynamic allocation, avoids an indirection accessing the
   // data, and exposes the resultant global to further GlobalOpt.
-  if (ConstantInt *NElements =
-              dyn_cast<ConstantInt>(getMallocArraySize(CI, Context, TD))) {
-    // Restrict this transformation to only working on small allocations
-    // (2048 bytes currently), as we don't want to introduce a 16M global or
-    // something.
-    if (TD && 
-        NElements->getZExtValue() * TD->getTypeAllocSize(AllocTy) < 2048) {
-      GVI = OptimizeGlobalAddressOfMalloc(GV, CI, BCI, Context, TD);
-      return true;
-    }
+  Value *NElems = getMallocArraySize(CI, Context, TD);
+  if (NElems) {
+    if (ConstantInt *NElements = dyn_cast<ConstantInt>(NElems))
+      // Restrict this transformation to only working on small allocations
+      // (2048 bytes currently), as we don't want to introduce a 16M global or
+      // something.
+      if (TD && 
+          NElements->getZExtValue() * TD->getTypeAllocSize(AllocTy) < 2048) {
+        GVI = OptimizeGlobalAddressOfMalloc(GV, CI, BCI, Context, TD);
+        return true;
+      }
   }
   
   // If the allocation is an array of structures, consider transforming this





More information about the llvm-commits mailing list