<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small">Clarification: this patch was authored by Cherry Zhang <<a href="mailto:cherryyz@google.com">cherryyz@google.com</a>>, I submitted on Cherry's behalf.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Apologies, I should have edited the commit message properly.</div><div class="gmail_default" style="font-size:small"><br>Thanks, Than</div><div class="gmail_default" style="font-size:small"><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Fri, Nov 16, 2018 at 9:30 AM Than McIntosh via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">thanm committed rL347050: [IRVerifier] Allow StructRet in statepoint.<br>
<br>
[IRVerifier] Allow StructRet in statepoint<br>
<br>
Summary:<br>
StructRet attribute is not allowed in vararg calls. The statepoint<br>
intrinsic is vararg, but the wrapped function may be not. Allow<br>
calls of statepoint with StructRet arg, as long as the wrapped<br>
function is not vararg.<br>
<br>
Reviewers: thanm, anna<br>
<br>
Reviewed By: anna<br>
<br>
Subscribers: anna, llvm-commits<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D53602" rel="noreferrer" target="_blank">https://reviews.llvm.org/D53602</a><br>
<br>
<br>
Files:<br>
  /llvm/trunk/lib/IR/Verifier.cpp<br>
  /llvm/trunk/test/Verifier/statepoint.ll<br>
<br>
PATCH<br>
<br>
Index: llvm/trunk/lib/IR/Verifier.cpp<br>
===================================================================<br>
--- llvm/trunk/lib/IR/Verifier.cpp      (revision 347049)<br>
+++ llvm/trunk/lib/IR/Verifier.cpp      (revision 347050)<br>
@@ -1930,6 +1930,7 @@<br>
<br>
   // Verify that the types of the call parameter arguments match<br>
   // the type of the wrapped callee.<br>
+  AttributeList Attrs = CS.getAttributes();<br>
   for (int i = 0; i < NumParams; i++) {<br>
     Type *ParamType = TargetFuncType->getParamType(i);<br>
     Type *ArgType = CS.getArgument(5 + i)->getType();<br>
@@ -1937,6 +1938,12 @@<br>
            "gc.statepoint call argument does not match wrapped "<br>
            "function type",<br>
            &CI);<br>
+<br>
+    if (TargetFuncType->isVarArg()) {<br>
+      AttributeSet ArgAttrs = Attrs.getParamAttributes(5 + i);<br>
+      Assert(!ArgAttrs.hasAttribute(Attribute::StructRet),<br>
+             "Attribute 'sret' cannot be used for vararg call arguments!", &CI);<br>
+    }<br>
   }<br>
<br>
   const int EndCallArgsInx = 4 + NumCallArgs;<br>
@@ -2814,8 +2821,13 @@<br>
         SawReturned = true;<br>
       }<br>
<br>
-      Assert(!ArgAttrs.hasAttribute(Attribute::StructRet),<br>
-             "Attribute 'sret' cannot be used for vararg call arguments!", I);<br>
+      // Statepoint intrinsic is vararg but the wrapped function may be not.<br>
+      // Allow sret here and check the wrapped function in verifyStatepoint.<br>
+      if (CS.getCalledFunction() == nullptr ||<br>
+          CS.getCalledFunction()->getIntrinsicID() !=<br>
+            Intrinsic::experimental_gc_statepoint)<br>
+        Assert(!ArgAttrs.hasAttribute(Attribute::StructRet),<br>
+               "Attribute 'sret' cannot be used for vararg call arguments!", I);<br>
<br>
       if (ArgAttrs.hasAttribute(Attribute::InAlloca))<br>
         Assert(Idx == CS.arg_size() - 1, "inalloca isn't on the last argument!",<br>
Index: llvm/trunk/test/Verifier/statepoint.ll<br>
===================================================================<br>
--- llvm/trunk/test/Verifier/statepoint.ll      (revision 347049)<br>
+++ llvm/trunk/test/Verifier/statepoint.ll      (revision 347050)<br>
@@ -4,6 +4,7 @@<br>
 declare i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token, i32, i32)<br>
 declare i64 addrspace(1)* @llvm.experimental.gc.relocate.p1i64(token, i32, i32)<br>
 declare token @llvm.experimental.gc.statepoint.p0f_isVoidf(i64, i32, void ()*, i32, i32, ...)<br>
+declare token @llvm.experimental.gc.statepoint.p0f_isVoidp0s_structsf(i64, i32, void (%struct*)*, i32, i32, ...)<br>
 declare i32 @"personality_function"()<br>
<br>
 ;; Basic usage<br>
@@ -79,3 +80,20 @@<br>
   %obj1.relocated1 = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %landing_pad, i32 12, i32 12)<br>
   ret i8 addrspace(1)* %obj1.relocated1<br>
 }<br>
+<br>
+; Test for statepoint with sret attribute.<br>
+; This should be allowed as long as the wrapped function is not vararg.<br>
+%struct = type { i64, i64, i64 }<br>
+<br>
+declare void @fn_sret(%struct* sret)<br>
+<br>
+define void @test_sret() gc "statepoint-example" {<br>
+  %x = alloca %struct<br>
+  %statepoint_token = call token (i64, i32, void (%struct*)*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidp0s_structsf(i64 0, i32 0, void (%struct*)* @fn_sret, i32 1, i32 0, %struct* sret %x, i32 0, i32 0)<br>
+  ret void<br>
+  ; CHECK-LABEL: test_sret<br>
+  ; CHECK: alloca<br>
+  ; CHECK: statepoint<br>
+  ; CHECK-SAME: sret<br>
+  ; CHECK: ret<br>
+}<br>
<br>
Users:<br>
  thanm (Author)<br>
<br>
<a href="https://reviews.llvm.org/rL347050" rel="noreferrer" target="_blank">https://reviews.llvm.org/rL347050</a><br>
<br>
<br>
<br>
</blockquote></div></div>