Don't constant fold through zero-length fields

[tjablin]

Summary

Previously, ConstantFoldLoadThroughGEPIndices and ConstantFoldLoadThroughGEPConstantExpr would return zeroinitializer when a constant GEP expression indexed to a zero-length field. The patch adds checks to these two methods based on isEmptyTy to avoid these scenarios. The patch also includes a simple testcase. I have verified that the testcase fails with r216246 and passes after applying the patch.

[tjablin]

Fix a related bug in ConstantFoldLoadThroughBitcast and add an additional test case.

[tjablin]

Ping

[hfinkel]

Could you please upload a full-context diff as described here: http://llvm.org/docs/Phabricator.html

If this is well-defined behavior (I realize that indexing off the end of an array is allowed, but whether that is permitted to alias other structure members is unclear to me), then I don't see why an empty type is a special case here. Shouldn't we have the same problem with any off-the-end array access where the array is not the last aggregate member?

test/Transforms/GlobalOpt/empty-type-field.ll

4

Remove this line (it is unneeded).

[tjablin]

I've updated the patch by replacing the CHECK-NOT test cases with CHECK test cases and included a new test demonstrating that indexing past the end of an array that is not the last field in a structure is already handled correctly.

[chandlerc]

Leaving this to Hal, I don't understand the specific bug you're fixing well enough to be a good reviewer.

lib/Analysis/ConstantFolding.cpp

do {
     // and find a bitcastable constant.
     if (!SrcTy->isAggregateType())
       return nullptr;
 
     // We're simulating a load through a pointer that was bitcast to point to
     // a different type, so we can try to walk down through the initial
     // elements of an aggregate to see if some part of th e aggregate is
     // castable to implement the "load" semantic model.
-    C = C->getAggregateElement(0u);
+    Constant *D = C->getAggregateElement(0U);
+    for (unsigned i = 0; D && D->getType()->isEmptyTy(); ++i)
+      D = C->getAggregateElement(i);
+    C = D;
   } while (C);
 
   return nullptr;
 }
 
 /// ConstantFoldLoadFromConstPtr - Return the value that a load from C would
 /// produce if it is constant and determinable.  If this is not determinable,
 /// return null.
if (!CE->getOperand(1)->isNullValue())
     return nullptr;  // Do not allow stepping over the value!
 
   // Loop over all of the operands, tracking down which value we are
   // addressing.
   for (unsigned i = 2, e = CE->getNumOperands(); i != e; ++i) {
     C = C->getAggregateElement(CE->getOperand(i));
     if (!C)
       return nullptr;
+
+    // Don't step through empty types. The aliasing is too hard to figure out.
+    if (C->getType()->isEmptyTy())
+      return nullptr;
   }
   return C;
 }
 
 /// ConstantFoldLoadThroughGEPIndices - Given a constant and getelementptr
 /// indices (with an *implied* zero pointer index that is not in the list),
 /// return the constant value being addressed by a virtual load, or null if
 /// something is funny and we can't decide.
 Constant *llvm::ConstantFoldLoadThroughGEPIndices(Constant *C,
                                                   ArrayRef<Constant*> Indices) {
   // Loop over all of the operands, tracking down which value we are
   // addressing.
   for (unsigned i = 0, e = Indices.size(); i != e; ++i) {
     C = C->getAggregateElement(Indices[i]);
     if (!C)
       return nullptr;
+
+    // Don't step through empty types. The aliasing is too hard to figure out.
+    if (C->getType()->isEmptyTy())
+      return nullptr;
   }
   return C;
 }
 
 
 //===----------------------------------------------------------------------===//
 //  Constant Folding for Calls
 //

test/Transforms/GlobalOpt/empty-type-field.ll

+; RUN: opt < %s -S -globalopt -instcombine | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%struct.qux = type { [0 x double], double }
+
+@qux = internal global %struct.qux { [0 x double] zeroinitializer, double 4.053854e+04 }, align 8
+
+%struct.gorp = type { [0 x i8*], i8* }
+
+@gorp = internal unnamed_addr constant %struct.gorp { [0 x i8*] zeroinitializer, i8* inttoptr (i64 1 to i8*) }, align 8
+
+%struct.norb = type { [1 x double], double }
+
+@norb = internal global %struct.norb { [1 x double] zeroinitializer, double 4.053854e+04 }, align 8
+
+define void @foo() {
+entry:
+  %0 = load double* getelementptr inbounds (%struct.qux* @qux, i32 0, i32 0, i32 0), align 1
+  call void @bar(double %0)
+; CHECK: call void @bar(double 4.053854e+04)
+
+  %1 = load i64* bitcast (%struct.gorp* @gorp to i64*), align 1
+  call fastcc void @blarg(i64 %1)
+; CHECK: call fastcc void @blarg(i64 1)
+
+  %2 = load double* getelementptr inbounds (%struct.norb* @norb, i32 0, i32 0, i32 1), align 1
+  call void @bar(double %0)
+; CHECK: call void @bar(double 4.053854e+04)
+
+  ret void
+}
+
+declare void @bar(double)
+declare void @blarg(i64)
 No newline at end of file