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

[Reid Kleckner]

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 <
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131204/b11c056e/attachment.html>