SROA: Handle a case of store size being smaller than allocation size

[rnk]

Summary

In this case, we are creating an x86_fp80 slice for a union from C where
the padding bytes may contain real data. An x86_fp80 alloca is 16 bytes,
and that's just fine. We can't, however, use regular loads and stores to
access the slice, because the store size is only 10 bytes / 80 bits.
Instead, use memcpy and memset.

Fixes PR18726.

[tjablin]

I think this patch might not be the right approach. The underlying issue is
that clang translates:

union U { long m0[3]; long double m1; };

to:

%union.U = type { x86_fp80, [16 x i8] }

The x86_fp80 type is 10 bytes of data aligned to 16 bytes. Is it
permissible to copy only the lower 10 bytes of an x86_fp80? I think the
answer should be yes. This is the behavior of the underlying x87 fldt and
fstpt instructions. If copying only the lower 10 bytes of a x86_fp80 is
permissible, then the C union was not correctly lowered to LLVM IR. I think
the correct resolution is to lower the union to:

%union.U = type { [4 x i64] }

[chandlerc]

Looks good with a slightly compactified test.

test/Transforms/SROA/slice-width.ll

65 ↗

(On Diff #12806)

Just test memset in the function above? We don't run other optimizations here, it's not going to forward or do dead store elim...

[rnk]

test/Transforms/SROA/slice-width.ll

65 ↗

(On Diff #12806)

I don't need to copy anything out, but I do need to load the i64 in order to cause SROA to fire. I guess I'll stick to that in both test cases.

[rnk]

Closed by commit rL216248 (authored by @rnk).

llvm/trunk/lib/Transforms/Scalar/SROA.cpp

bool visitMemSetInst(MemSetInst &II) {
     Type *AllocaTy = NewAI.getAllocatedType();
     Type *ScalarTy = AllocaTy->getScalarType();
 
     // If this doesn't map cleanly onto the alloca type, and that type isn't
     // a single value type, just emit a memset.
     if (!VecTy && !IntTy &&
         (BeginOffset > NewAllocaBeginOffset ||
          EndOffset < NewAllocaEndOffset ||
+         SliceSize != DL.getTypeStoreSize(AllocaTy) ||
          !AllocaTy->isSingleValueType() ||
          !DL.isLegalInteger(DL.getTypeSizeInBits(ScalarTy)) ||
          DL.getTypeSizeInBits(ScalarTy)%8 != 0)) {
       Type *SizeTy = II.getLength()->getType();
       Constant *Size = ConstantInt::get(SizeTy, NewEndOffset - NewBeginOffset);
       CallInst *New = IRB.CreateMemSet(
           getNewAllocaSlicePtr(IRB, OldPtr->getType()), II.getValue(), Size,
           getSliceAlign(), II.isVolatile());
bool visitMemTransferInst(MemTransferInst &II) {
     // For split transfer intrinsics we have an incredibly useful assurance:
     // the source and destination do not reside within the same alloca, and at
     // least one of them does not escape. This means that we can replace
     // memmove with memcpy, and we don't need to worry about all manner of
     // downsides to splitting and transforming the operations.
 
     // If this doesn't map cleanly onto the alloca type, and that type isn't
     // a single value type, just emit a memcpy.
-    bool EmitMemCpy
-      = !VecTy && !IntTy && (BeginOffset > NewAllocaBeginOffset ||
-                             EndOffset < NewAllocaEndOffset ||
-                             !NewAI.getAllocatedType()->isSingleValueType());
+    bool EmitMemCpy =
+        !VecTy && !IntTy &&
+        (BeginOffset > NewAllocaBeginOffset || EndOffset < NewAllocaEndOffset ||
+         SliceSize != DL.getTypeStoreSize(NewAI.getAllocatedType()) ||
 
     // If we're just going to emit a memcpy, the alloca hasn't changed, and the
     // size hasn't been shrunk based on analysis of the viable range, this is
     // a no-op.
     if (EmitMemCpy && &OldAI == &NewAI) {
       // Ensure the start lines up.
       assert(NewBeginOffset == BeginOffset);
 

llvm/trunk/test/Transforms/SROA/slice-width.ll

 ; RUN: opt < %s -sroa -S | FileCheck %s
-target datalayout = "e-p:64:64:64-p1:16:16:16-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-n8:16:32:64"
+target datalayout = "e-p:64:64:64-p1:16:16:16-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:64:64-f80:128-v64:64:64-v128:128:128-a0:0:64-n8:16:32:64"
 
 declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture, i8* nocapture, i32, i32, i1) nounwind
+declare void @llvm.memset.p0i8.i32(i8* nocapture, i8, i32, i32, i1) nounwind
 
 define void @no_split_on_non_byte_width(i32) {
 ; This tests that allocas are not split into slices that are not byte width multiple
   %arg = alloca i32 , align 8
   store i32 %0, i32* %arg
   br label %load_i32
 
 load_i32:
 ; CHECK-LABEL: load_i32:
 ; CHECK-NOT: bitcast {{.*}} to i1
 ; CHECK-NOT: zext i1
   %r0 = load i32* %arg
   br label %load_i1
 
 load_i1:
 ; CHECK-LABEL: load_i1:
 ; CHECK: bitcast {{.*}} to i1
   %p1 = bitcast i32* %arg to i1*
   %t1 = load i1* %p1
   ret void
 }
+
+; PR18726: Check that we use memcpy and memset to fill out padding when we have
+; a slice with a simple single type whose store size is smaller than the slice
+; size.
+
+%union.Foo = type { x86_fp80, i64, i64 }
+
+@foo_copy_source = external constant %union.Foo
+@i64_sink = global i64 0
+
+define void @memcpy_fp80_padding() {
+  %x = alloca %union.Foo
+
+  ; Copy from a global.
+  %x_i8 = bitcast %union.Foo* %x to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %x_i8, i8* bitcast (%union.Foo* @foo_copy_source to i8*), i32 32, i32 16, i1 false)
+
+  ; Access a slice of the alloca to trigger SROA.
+  %mid_p = getelementptr %union.Foo* %x, i32 0, i32 1
+  %elt = load i64* %mid_p
+  store i64 %elt, i64* @i64_sink
+  ret void
+}
+; CHECK-LABEL: define void @memcpy_fp80_padding
+; CHECK: alloca x86_fp80
+; CHECK: call void @llvm.memcpy.p0i8.p0i8.i32
+; CHECK: load i64* getelementptr inbounds (%union.Foo* @foo_copy_source, i64 0, i32 1)
+; CHECK: load i64* getelementptr inbounds (%union.Foo* @foo_copy_source, i64 0, i32 2)
+
+define void @memset_fp80_padding() {
+  %x = alloca %union.Foo
+
+  ; Set to all ones.
+  %x_i8 = bitcast %union.Foo* %x to i8*
+  call void @llvm.memset.p0i8.i32(i8* %x_i8, i8 -1, i32 32, i32 16, i1 false)
+
+  ; Access a slice of the alloca to trigger SROA.
+  %mid_p = getelementptr %union.Foo* %x, i32 0, i32 1
+  %elt = load i64* %mid_p
+  store i64 %elt, i64* @i64_sink
+  ret void
+}
+; CHECK-LABEL: define void @memset_fp80_padding
+; CHECK: alloca x86_fp80
+; CHECK: call void @llvm.memset.p0i8.i32(i8* %{{.*}}, i8 -1, i32 16, i32 16, i1 false)
+; CHECK: store i64 -1, i64* @i64_sink