<div dir="ltr"><div><div><div>I think this patch might not be the right approach. The underlying issue is that clang translates:<br><br>union U { long m0[3]; long double m1; };<br><br></div>to:<br><br>%union.U = type { x86_fp80, [16 x i8] }<br>
<br></div>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:<br>
<br></div>%union.U = type { [4 x i64] }<br><br><br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Aug 21, 2014 at 4:37 PM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi chandlerc,<br>
<br>
In this case, we are creating an x86_fp80 slice for a union from C where<br>
the padding bytes may contain real data. An x86_fp80 alloca is 16 bytes,<br>
and that's just fine. We can't, however, use regular loads and stores to<br>
access the slice, because the store size is only 10 bytes / 80 bits.<br>
Instead, use memcpy and memset.<br>
<br>
Fixes PR18726.<br>
<br>
<a href="http://reviews.llvm.org/D5012" target="_blank">http://reviews.llvm.org/D5012</a><br>
<br>
Files:<br>
lib/Transforms/Scalar/SROA.cpp<br>
test/Transforms/SROA/slice-width.ll<br>
<br>
Index: lib/Transforms/Scalar/SROA.cpp<br>
===================================================================<br>
--- lib/Transforms/Scalar/SROA.cpp<br>
+++ lib/Transforms/Scalar/SROA.cpp<br>
@@ -2422,6 +2422,7 @@<br>
if (!VecTy && !IntTy &&<br>
(BeginOffset > NewAllocaBeginOffset ||<br>
EndOffset < NewAllocaEndOffset ||<br>
+ SliceSize != DL.getTypeStoreSize(AllocaTy) ||<br>
!AllocaTy->isSingleValueType() ||<br>
!DL.isLegalInteger(DL.getTypeSizeInBits(ScalarTy)) ||<br>
DL.getTypeSizeInBits(ScalarTy)%8 != 0)) {<br>
@@ -2544,10 +2545,11 @@<br>
<br>
// If this doesn't map cleanly onto the alloca type, and that type isn't<br>
// a single value type, just emit a memcpy.<br>
- bool EmitMemCpy<br>
- = !VecTy && !IntTy && (BeginOffset > NewAllocaBeginOffset ||<br>
- EndOffset < NewAllocaEndOffset ||<br>
- !NewAI.getAllocatedType()->isSingleValueType());<br>
+ bool EmitMemCpy =<br>
+ !VecTy && !IntTy &&<br>
+ (BeginOffset > NewAllocaBeginOffset || EndOffset < NewAllocaEndOffset ||<br>
+ SliceSize != DL.getTypeStoreSize(NewAI.getAllocatedType()) ||<br>
+ !NewAI.getAllocatedType()->isSingleValueType());<br>
<br>
// If we're just going to emit a memcpy, the alloca hasn't changed, and the<br>
// size hasn't been shrunk based on analysis of the viable range, this is<br>
Index: test/Transforms/SROA/slice-width.ll<br>
===================================================================<br>
--- test/Transforms/SROA/slice-width.ll<br>
+++ test/Transforms/SROA/slice-width.ll<br>
@@ -1,7 +1,8 @@<br>
; RUN: opt < %s -sroa -S | FileCheck %s<br>
-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"<br>
+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"<br>
<br>
declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture, i8* nocapture, i32, i32, i1) nounwind<br>
+declare void @llvm.memset.p0i8.i32(i8* nocapture, i8, i32, i32, i1) nounwind<br>
<br>
define void @no_split_on_non_byte_width(i32) {<br>
; This tests that allocas are not split into slices that are not byte width multiple<br>
@@ -23,3 +24,57 @@<br>
%t1 = load i1* %p1<br>
ret void<br>
}<br>
+<br>
+; PR18726: Check that we use memcpy and memset to fill out padding when we have<br>
+; a slice with a simple single type whose store size is smaller than the slice<br>
+; size.<br>
+<br>
+%union.Foo = type { x86_fp80, i64, i64 }<br>
+<br>
+@foo_copy_source = external constant %union.Foo<br>
+@i64_sink = global i64 0<br>
+<br>
+define void @memcpy_fp80_padding(%union.Foo* sret %res) {<br>
+ %x = alloca %union.Foo<br>
+<br>
+ ; Copy from a global.<br>
+ %x_i8 = bitcast %union.Foo* %x to i8*<br>
+ 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)<br>
+<br>
+ ; Access a slice of the alloca to trigger SROA.<br>
+ %mid_p = getelementptr %union.Foo* %x, i32 0, i32 1<br>
+ %elt = load i64* %mid_p<br>
+ store i64 %elt, i64* @i64_sink<br>
+<br>
+ ; Copy out.<br>
+ %res_i8 = bitcast %union.Foo* %res to i8*<br>
+ call void @llvm.memcpy.p0i8.p0i8.i32(i8* %res_i8, i8* %x_i8, i32 32, i32 16, i1 false)<br>
+ ret void<br>
+}<br>
+; CHECK-LABEL: define void @memcpy_fp80_padding<br>
+; CHECK: alloca x86_fp80<br>
+; CHECK: call void @llvm.memcpy.p0i8.p0i8.i32<br>
+; CHECK: load i64* getelementptr inbounds (%union.Foo* @foo_copy_source, i64 0, i32 1)<br>
+; CHECK: load i64* getelementptr inbounds (%union.Foo* @foo_copy_source, i64 0, i32 2)<br>
+<br>
+define void @memset_fp80_padding(%union.Foo* sret %res) {<br>
+ %x = alloca %union.Foo<br>
+<br>
+ ; Set to all ones.<br>
+ %x_i8 = bitcast %union.Foo* %x to i8*<br>
+ call void @llvm.memset.p0i8.i32(i8* %x_i8, i8 -1, i32 32, i32 16, i1 false)<br>
+<br>
+ ; Access a slice of the alloca to trigger SROA.<br>
+ %mid_p = getelementptr %union.Foo* %x, i32 0, i32 1<br>
+ %elt = load i64* %mid_p<br>
+ store i64 %elt, i64* @i64_sink<br>
+<br>
+ ; Copy out.<br>
+ %res_i8 = bitcast %union.Foo* %res to i8*<br>
+ call void @llvm.memcpy.p0i8.p0i8.i32(i8* %res_i8, i8* %x_i8, i32 32, i32 16, i1 false)<br>
+ ret void<br>
+}<br>
+; CHECK-LABEL: define void @memset_fp80_padding<br>
+; CHECK: alloca x86_fp80<br>
+; CHECK: call void @llvm.memset.p0i8.i32(i8* %{{.*}}, i8 -1, i32 16, i32 16, i1 false)<br>
+; CHECK: store i64 -1, i64* @i64_sink<br>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>