<div dir="ltr">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.</div><div class="gmail_extra"><br><br><div class="gmail_quote">
On Wed, Dec 4, 2013 at 1:59 AM, Richard Sandiford <span dir="ltr"><<a href="mailto:rsandifo@linux.vnet.ibm.com" target="_blank">rsandifo@linux.vnet.ibm.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: rsandifo<br>
Date: Wed Dec  4 03:59:57 2013<br>
New Revision: 196370<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=196370&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=196370&view=rev</a><br>
Log:<br>
[SystemZ] Fix handling of pass-by-pointer arguments<br>
<br>
I'd misunderstood getIndirect() to mean that the argument should be passed<br>
as a pointer at the ABI level, with the ByVal argument choosing caller-copy<br>
semantics over no-caller-copy (callee-copy-on-write) semantics.  But<br>
getIndirect(x) actually means that x is passed by pointer at the IR<br>
level but (at least on all other targets I looked at) directly at the<br>
ABI level.  getIndirect(x, false) selects a pointer to a caller-made<br>
copy, which is what SystemZ was aiming for.<br>
<br>
This fixes a miscompilation of c-index-test.  Structure arguments were being<br>
passed by pointer, but no copy was being made, so a write in the callee<br>
stomped over a caller's local variable.<br>
<br>
Modified:<br>
    cfe/trunk/lib/CodeGen/TargetInfo.cpp<br>
    cfe/trunk/test/CodeGen/systemz-inline-asm.c<br>
<br>
Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=196370&r1=196369&r2=196370&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=196370&r1=196369&r2=196370&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)<br>
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Wed Dec  4 03:59:57 2013<br>
@@ -4573,7 +4573,7 @@ ABIArgInfo SystemZABIInfo::classifyArgum<br>
   // Values that are not 1, 2, 4 or 8 bytes in size are passed indirectly.<br>
   uint64_t Size = getContext().getTypeSize(Ty);<br>
   if (Size != 8 && Size != 16 && Size != 32 && Size != 64)<br>
-    return ABIArgInfo::getIndirect(0);<br>
+    return ABIArgInfo::getIndirect(0, /*ByVal=*/false);<br>
<br>
   // Handle small structures.<br>
   if (const RecordType *RT = Ty->getAs<RecordType>()) {<br>
@@ -4581,7 +4581,7 @@ ABIArgInfo SystemZABIInfo::classifyArgum<br>
     // fail the size test above.<br>
     const RecordDecl *RD = RT->getDecl();<br>
     if (RD->hasFlexibleArrayMember())<br>
-      return ABIArgInfo::getIndirect(0);<br>
+      return ABIArgInfo::getIndirect(0, /*ByVal=*/false);<br>
<br>
     // The structure is passed as an unextended integer, a float, or a double.<br>
     llvm::Type *PassTy;<br>
@@ -4598,7 +4598,7 @@ ABIArgInfo SystemZABIInfo::classifyArgum<br>
<br>
   // Non-structure compounds are passed indirectly.<br>
   if (isCompoundType(Ty))<br>
-    return ABIArgInfo::getIndirect(0);<br>
+    return ABIArgInfo::getIndirect(0, /*ByVal=*/false);<br>
<br>
   return ABIArgInfo::getDirect(0);<br>
 }<br>
<br>
Modified: cfe/trunk/test/CodeGen/systemz-inline-asm.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/systemz-inline-asm.c?rev=196370&r1=196369&r2=196370&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/systemz-inline-asm.c?rev=196370&r1=196369&r2=196370&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/test/CodeGen/systemz-inline-asm.c (original)<br>
+++ cfe/trunk/test/CodeGen/systemz-inline-asm.c Wed Dec  4 03:59:57 2013<br>
@@ -123,7 +123,7 @@ double test_f64(double f, double g) {<br>
 long double test_f128(long double f, long double g) {<br>
   asm("axbr %0, %2" : "=f" (f) : "0" (f), "f" (g));<br>
   return f;<br>
-// CHECK: define void @test_f128(fp128* noalias nocapture sret [[DEST:%.*]], fp128* byval nocapture readonly, fp128* byval nocapture readonly)<br>
+// CHECK: define void @test_f128(fp128* noalias nocapture sret [[DEST:%.*]], fp128* nocapture readonly, fp128* nocapture readonly)<br>
 // CHECK: %f = load fp128* %0<br>
 // CHECK: %g = load fp128* %1<br>
 // CHECK: [[RESULT:%.*]] = tail call fp128 asm "axbr $0, $2", "=f,0,f"(fp128 %f, fp128 %g)<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>