[ConstantFolding] Handle leading zero-length elements in load folding

[nikic]

Summary

Struct types may have leading zero-length elements like [0 x i32], in which case the "real" element at offset 0 will not necessarily coincide with the 0th element of the aggregate. ConstantFoldLoadThroughBitcast() wants to drill down the element at offset 0, but currently always picks the 0th aggregate element to do so. This patch changes the code to find the first non-zero-length element instead.

The motivation behind this change is https://github.com/rust-lang/rust/issues/48627. Rust is fond of emitting [0 x iN] separators between struct elements to enforce alignment, which prevents constant folding in this particular case.

[nikic]

Ping

[spatel]

I don't have a good understanding of the potential edge cases with aggregates, so adding some other potential reviewers.

[efriedma]

lib/Analysis/ConstantFolding.cpp

352 ↗

(On Diff #176269)

This is probably an infinite loop on something like [4294967296 x [0 x i32]]. (An LLVM array can have up to 2^64 elements.) Not sure how much we care... it looks like there are overflows like this all over the place in LLVM.

Otherwise looks fine.

[nikic]

lib/Analysis/ConstantFolding.cpp

352 ↗

(On Diff #176269)

I tried

@g8 = constant [4294967296 x [0 x i32]] zeroinitializer
define i64 @test_leading_zero_size_elems_big2() {
  %v = load i64, i64* bitcast ([4294967296 x [0 x i32]]* @g8 to i64*)
  ret i64 %v
}

which did not result in an infinite loop ... because ConstantAggregateZero::getNumElements() also returns unsigned, so the number of elements is truncated to 0 :/

Still, using

@g8 = constant [4294967295 x [0 x i32]] zeroinitializer
define i64 @test_leading_zero_size_elems_big2() {
  %v = load i64, i64* bitcast ([4294967295 x [0 x i32]]* @g8 to i64*)
  ret i64 %v
}

ends up looping over array elements for no good reason, it's not like there is a change of finding a non-zero size elements by looking at further elements.

I think I'll change this code to handle the struct case separately, as I think it's the only one where this really makes sense.

[nikic]

Only skip zero-size elements for struct types. Add tests for large arrays of zero-size elements.

[nikic]

test/Transforms/ConstProp/loads.ll

309 ↗

(On Diff #177675)

This result is unrelated to this patch, but I'm wondering if it's correct. Is this folding to zero based on UB, as this is effectively an out-of-bounds access to a constant array?

[efriedma]

LGTM. (We should fix the constant stuff to be consistent with the type system at some point, but not urgent, I guess.)

test/Transforms/ConstProp/loads.ll

309 ↗

(On Diff #177675)

Yes, it's ignoring the offset because any well-defined load from a zeroinitializer constant must return zero.

llvm/trunk/lib/Analysis/ConstantFolding.cpp

do {
 
     // If this isn't an aggregate type, there is nothing we can do to drill down
     // 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
+    // elements of an aggregate to see if some part of the aggregate is
     // castable to implement the "load" semantic model.
+    if (SrcTy->isStructTy()) {
+      // Struct types might have leading zero-length elements like [0 x i32],
+      // which are certainly not what we are looking for, so skip them.
+      unsigned Elem = 0;
+      Constant *ElemC;
+      do {
+        ElemC = C->getAggregateElement(Elem++);
+      } while (ElemC && DL.getTypeSizeInBits(ElemC->getType()) == 0);
+      C = ElemC;
+    } else {
       C = C->getAggregateElement(0u);
+    }
   } while (C);
 
   return nullptr;
 }
 
 namespace {
 
 /// Recursive helper to read bits out of global. C is the constant being copied

llvm/trunk/test/Transforms/ConstProp/loads.ll

define i64 @test16.3() {
   ret i64 %v
 
 ; LE-LABEL: @test16.3(
 ; LE: ret i64 0
 
 ; BE-LABEL: @test16.3(
 ; BE: ret i64 0
 }
+
+@g7 = constant {[0 x i32], [0 x i8], {}*} { [0 x i32] undef, [0 x i8] undef, {}* null }
+
+define i64* @test_leading_zero_size_elems() {
+  %v = load i64*, i64** bitcast ({[0 x i32], [0 x i8], {}*}* @g7 to i64**)
+  ret i64* %v
+
+; LE-LABEL: @test_leading_zero_size_elems(
+; LE: ret i64* null
+
+; BE-LABEL: @test_leading_zero_size_elems(
+; BE: ret i64* null
+}
+
+@g8 = constant {[4294967295 x [0 x i32]], i64} { [4294967295 x [0 x i32]] undef, i64 123 }
+
+define i64 @test_leading_zero_size_elems_big() {
+  %v = load i64, i64* bitcast ({[4294967295 x [0 x i32]], i64}* @g8 to i64*)
+  ret i64 %v
+
+; LE-LABEL: @test_leading_zero_size_elems_big(
+; LE: ret i64 123
+
+; BE-LABEL: @test_leading_zero_size_elems_big(
+; BE: ret i64 123
+}
+
+@g9 = constant [4294967295 x [0 x i32]] zeroinitializer
+
+define i64 @test_array_of_zero_size_array() {
+  %v = load i64, i64* bitcast ([4294967295 x [0 x i32]]* @g9 to i64*)
+  ret i64 %v
+
+; LE-LABEL: @test_array_of_zero_size_array(
+; LE: ret i64 0
+
+; BE-LABEL: @test_array_of_zero_size_array(
+; BE: ret i64 0
+}