r196370 - [SystemZ] Fix handling of pass-by-pointer arguments

[Richard Sandiford]

Reid Kleckner <rnk at google.com> writes:
> 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.

Sounds good to me FWIW (although I've not got time to work on it...)

Thanks,
Richard
>
>
> On Wed, Dec 4, 2013 at 1:59 AM, Richard Sandiford <
> rsandifo at linux.vnet.ibm.com> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>