Akira Hatanaka
2011-Jul-29 18:13 UTC
[LLVMdev] alignment checking in isSafeToEliminateVarargsCast
I have a question about a problem I came across while I was adding support
for aggregate va_arg expression in clang.
The following is the example program I will use in this email. I compile the
program with clang targeting mips. Note that I have not pushed all the
changes I have made yet, so you will not be able to see the same results.
$ clang -ccc-host-triple mipsel-unknown-linux -ccc-clang-archs mipsel foo.c
-o foo.ll -emit-llvm -O3 -S
// foo.c
// adapted from test-suite/SingleSource/UnitTests/2003-05-07-VarArg
s.c
#include <stdarg.h>
typedef struct DWordS_struct { int i; char c; } DWordS;
typedef struct LargeS_struct { int i; double d; DWordS* ptr; int j; }
LargeS;
void test(char *fmt,...);
int main() {
DWordS dw = { 18, 'a' };
LargeS ls = { 21, 22.0, &dw, 23 };
test("DQL", dw, ls);
return 0;
}
After code generation, clang runs several llvm IR optimization passes
including InstCombine.
The following is the bitcode before InstCombine is run. The second argument
%dw of function test is of type "%struct.DWordS_struct* byval", which
has a
4-byte alignment.
Breakpoint 1 at
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1652
1652 InstCombineIRInserter(Worklist));
(gdb) p F.dump()
define i32 @main() nounwind {
entry:
%dw = alloca %struct.DWordS_struct, align 4
%ls = alloca %struct.LargeS_struct, align 8
%tmp = bitcast %struct.DWordS_struct* %dw to i8*
call void @llvm.memcpy.p0i8.p0i8.i32(i8* %tmp, i8* bitcast ({ i32, i8, [3
x i8] }* @main.dw to i8*), i32 8, i32 4, i1 false)
%i = getelementptr inbounds %struct.LargeS_struct* %ls, i32 0, i32 0
store i32 21, i32* %i, align 4, !tbaa !0
%d = getelementptr inbounds %struct.LargeS_struct* %ls, i32 0, i32 1
store double 2.200000e+01, double* %d, align 8, !tbaa !3
%ptr = getelementptr inbounds %struct.LargeS_struct* %ls, i32 0, i32 2
store %struct.DWordS_struct* %dw, %struct.DWordS_struct** %ptr, align 4,
!tbaa !4
%j = getelementptr inbounds %struct.LargeS_struct* %ls, i32 0, i32 3
store i32 23, i32* %j, align 4, !tbaa !0
call void (i8*, ...)* @test(i8* getelementptr inbounds ([4 x i8]* @.str,
i32 0, i32 0), %struct.DWordS_struct* byval %dw, %struct.LargeS_struct*
byval %ls)
ret i32 0
}
After InstCombine is run, the type of the second argument of function test
has been changed to "i64* byval", which has an 8-byte alignment. This
becomes a problem if the target is mips-o32, since a function call uses
different registers or different stack locations to pass arguments depending
on the alignment of the type of the argument that is passed.
Breakpoint 2 at
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1667
1667 return EverMadeChange;
(gdb) p F.dump()
define i32 @main() nounwind {
entry:
%dw = alloca i64, align 8
%tmpcast = bitcast i64* %dw to %struct.DWordS_struct*
%ls = alloca %struct.LargeS_struct, align 8
store i64 416611827730, i64* %dw, align 8
%i = getelementptr inbounds %struct.LargeS_struct* %ls, i32 0, i32 0
store i32 21, i32* %i, align 8, !tbaa !0
%d = getelementptr inbounds %struct.LargeS_struct* %ls, i32 0, i32 1
store double 2.200000e+01, double* %d, align 8, !tbaa !3
%ptr = getelementptr inbounds %struct.LargeS_struct* %ls, i32 0, i32 2
store %struct.DWordS_struct* %tmpcast, %struct.DWordS_struct** %ptr, align
8, !tbaa !4
%j = getelementptr inbounds %struct.LargeS_struct* %ls, i32 0, i32 3
store i32 23, i32* %j, align 4, !tbaa !0
call void (i8*, ...)* @test(i8* getelementptr inbounds ([4 x i8]* @.str,
i32 0, i32 0), i64* byval %dw, %struct.LargeS_struct* byval %ls) nounwind
ret i32 0
}
This transformation seems to take place in InstCombiner::visitCallSite in
InstCombineCalls.cpp, which calls function isSafeToEliminateVarargsCast to
decide whether it is safe to eliminate a cast instruction and replace the
argument that is passed. Would it be possible to add code in
isSafeToEliminateVarargsCast that checks alignments of SrcTy and DstTy and
returns false if they are not the same? Or are there better ways to
circumvent this problem?
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20110729/825cf9bb/attachment.html>
Eli Friedman
2011-Jul-29 19:05 UTC
[LLVMdev] alignment checking in isSafeToEliminateVarargsCast
On Fri, Jul 29, 2011 at 11:13 AM, Akira Hatanaka <ahatanak at gmail.com> wrote:> I have a question about a problem I came across while I was adding support > for aggregate va_arg expression in clang. > The following is the example program I will use in this email. I compile the > program with clang targeting mips. Note that I have not pushed all the > changes I have made yet, so you will not be able to see the same results. > > $ clang -ccc-host-triple mipsel-unknown-linux -ccc-clang-archs mipsel foo.c > -o foo.ll -emit-llvm -O3 -S > // foo.c > // adapted from test-suite/SingleSource/UnitTests/2003-05-07-VarArg > s.c > #include <stdarg.h> > > typedef struct DWordS_struct { int i; char c; } DWordS; > typedef struct LargeS_struct { int i; double d; DWordS* ptr; int j; } > LargeS; > > void test(char *fmt,...); > > int main() { > DWordS dw = { 18, 'a' }; > LargeS ls = { 21, 22.0, &dw, 23 }; > > test("DQL", dw, ls); > > return 0; > } > > After code generation, clang runs several llvm IR optimization passes > including InstCombine. > The following is the bitcode before InstCombine is run. The second argument > %dw of function test is of type "%struct.DWordS_struct* byval", which has a > 4-byte alignment. > > > Breakpoint 1 at > llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1652 > 1652 InstCombineIRInserter(Worklist)); > (gdb) p F.dump() > > define i32 @main() nounwind { > entry: > %dw = alloca %struct.DWordS_struct, align 4 > %ls = alloca %struct.LargeS_struct, align 8 > %tmp = bitcast %struct.DWordS_struct* %dw to i8* > call void @llvm.memcpy.p0i8.p0i8.i32(i8* %tmp, i8* bitcast ({ i32, i8, [3 > x i8] }* @main.dw to i8*), i32 8, i32 4, i1 false) > %i = getelementptr inbounds %struct.LargeS_struct* %ls, i32 0, i32 0 > store i32 21, i32* %i, align 4, !tbaa !0 > %d = getelementptr inbounds %struct.LargeS_struct* %ls, i32 0, i32 1 > store double 2.200000e+01, double* %d, align 8, !tbaa !3 > %ptr = getelementptr inbounds %struct.LargeS_struct* %ls, i32 0, i32 2 > store %struct.DWordS_struct* %dw, %struct.DWordS_struct** %ptr, align 4, > !tbaa !4 > %j = getelementptr inbounds %struct.LargeS_struct* %ls, i32 0, i32 3 > store i32 23, i32* %j, align 4, !tbaa !0 > call void (i8*, ...)* @test(i8* getelementptr inbounds ([4 x i8]* @.str, > i32 0, i32 0), %struct.DWordS_struct* byval %dw, %struct.LargeS_struct* > byval %ls) > ret i32 0 > } > > > After InstCombine is run, the type of the second argument of function test > has been changed to "i64* byval", which has an 8-byte alignment. This > becomes a problem if the target is mips-o32, since a function call uses > different registers or different stack locations to pass arguments depending > on the alignment of the type of the argument that is passed. > > Breakpoint 2 at > llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1667 > 1667 return EverMadeChange; > (gdb) p F.dump() > define i32 @main() nounwind { > entry: > %dw = alloca i64, align 8 > %tmpcast = bitcast i64* %dw to %struct.DWordS_struct* > %ls = alloca %struct.LargeS_struct, align 8 > store i64 416611827730, i64* %dw, align 8 > %i = getelementptr inbounds %struct.LargeS_struct* %ls, i32 0, i32 0 > store i32 21, i32* %i, align 8, !tbaa !0 > %d = getelementptr inbounds %struct.LargeS_struct* %ls, i32 0, i32 1 > store double 2.200000e+01, double* %d, align 8, !tbaa !3 > %ptr = getelementptr inbounds %struct.LargeS_struct* %ls, i32 0, i32 2 > store %struct.DWordS_struct* %tmpcast, %struct.DWordS_struct** %ptr, align > 8, !tbaa !4 > %j = getelementptr inbounds %struct.LargeS_struct* %ls, i32 0, i32 3 > store i32 23, i32* %j, align 4, !tbaa !0 > call void (i8*, ...)* @test(i8* getelementptr inbounds ([4 x i8]* @.str, > i32 0, i32 0), i64* byval %dw, %struct.LargeS_struct* byval %ls) nounwind > ret i32 0 > } > > This transformation seems to take place in InstCombiner::visitCallSite in > InstCombineCalls.cpp, which calls function isSafeToEliminateVarargsCast to > decide whether it is safe to eliminate a cast instruction and replace the > argument that is passed. Would it be possible to add code in > isSafeToEliminateVarargsCast that checks alignments of SrcTy and DstTy and > returns false if they are not the same? Or are there better ways to > circumvent this problem?Looks like a legitimate bug. We really shouldn't be touching the types of byval arguments... -Eli