I also find this super confusing. IMO we should have a distinct ABIArgInfo kind for byval, and get rid of the ByVal default boolean parameter.
On Wed, Dec 4, 2013 at 1:59 AM, Richard Sandiford < [email protected]> wrote: > Author: rsandifo > Date: Wed Dec 4 03:59:57 2013 > New Revision: 196370 > > URL: http://llvm.org/viewvc/llvm-project?rev=196370&view=rev > Log: > [SystemZ] Fix handling of pass-by-pointer arguments > > I'd misunderstood getIndirect() to mean that the argument should be passed > as a pointer at the ABI level, with the ByVal argument choosing caller-copy > semantics over no-caller-copy (callee-copy-on-write) semantics. But > getIndirect(x) actually means that x is passed by pointer at the IR > level but (at least on all other targets I looked at) directly at the > ABI level. getIndirect(x, false) selects a pointer to a caller-made > copy, which is what SystemZ was aiming for. > > This fixes a miscompilation of c-index-test. Structure arguments were > being > passed by pointer, but no copy was being made, so a write in the callee > stomped over a caller's local variable. > > Modified: > cfe/trunk/lib/CodeGen/TargetInfo.cpp > cfe/trunk/test/CodeGen/systemz-inline-asm.c > > Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=196370&r1=196369&r2=196370&view=diff > > ============================================================================== > --- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original) > +++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Wed Dec 4 03:59:57 2013 > @@ -4573,7 +4573,7 @@ ABIArgInfo SystemZABIInfo::classifyArgum > // Values that are not 1, 2, 4 or 8 bytes in size are passed indirectly. > uint64_t Size = getContext().getTypeSize(Ty); > if (Size != 8 && Size != 16 && Size != 32 && Size != 64) > - return ABIArgInfo::getIndirect(0); > + return ABIArgInfo::getIndirect(0, /*ByVal=*/false); > > // Handle small structures. > if (const RecordType *RT = Ty->getAs<RecordType>()) { > @@ -4581,7 +4581,7 @@ ABIArgInfo SystemZABIInfo::classifyArgum > // fail the size test above. > const RecordDecl *RD = RT->getDecl(); > if (RD->hasFlexibleArrayMember()) > - return ABIArgInfo::getIndirect(0); > + return ABIArgInfo::getIndirect(0, /*ByVal=*/false); > > // The structure is passed as an unextended integer, a float, or a > double. > llvm::Type *PassTy; > @@ -4598,7 +4598,7 @@ ABIArgInfo SystemZABIInfo::classifyArgum > > // Non-structure compounds are passed indirectly. > if (isCompoundType(Ty)) > - return ABIArgInfo::getIndirect(0); > + return ABIArgInfo::getIndirect(0, /*ByVal=*/false); > > return ABIArgInfo::getDirect(0); > } > > Modified: cfe/trunk/test/CodeGen/systemz-inline-asm.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/systemz-inline-asm.c?rev=196370&r1=196369&r2=196370&view=diff > > ============================================================================== > --- cfe/trunk/test/CodeGen/systemz-inline-asm.c (original) > +++ cfe/trunk/test/CodeGen/systemz-inline-asm.c Wed Dec 4 03:59:57 2013 > @@ -123,7 +123,7 @@ double test_f64(double f, double g) { > long double test_f128(long double f, long double g) { > asm("axbr %0, %2" : "=f" (f) : "0" (f), "f" (g)); > return f; > -// CHECK: define void @test_f128(fp128* noalias nocapture sret > [[DEST:%.*]], fp128* byval nocapture readonly, fp128* byval nocapture > readonly) > +// CHECK: define void @test_f128(fp128* noalias nocapture sret > [[DEST:%.*]], fp128* nocapture readonly, fp128* nocapture readonly) > // CHECK: %f = load fp128* %0 > // CHECK: %g = load fp128* %1 > // CHECK: [[RESULT:%.*]] = tail call fp128 asm "axbr $0, $2", > "=f,0,f"(fp128 %f, fp128 %g) > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
