[INCOMPLETE] [GC] Support wrapping vararg functions in statepoint

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[INCOMPLETE] [GC] Support wrapping vararg functions in statepoint

Ramkumar Ramachandra
I actually need this feature quite badly in my untyped language
compiler: since I support first-class functions, I've made the types of
all functions a standard vararg (so I can box them).

The implementation crashes when I try to read out the value of
gc.result. Hints as to what might be wrong?

Signed-off-by: Ramkumar Ramachandra <[hidden email]>
---
 lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp |  4 ++--
 lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h   |  2 +-
 lib/CodeGen/SelectionDAG/StatepointLowering.cpp  | 11 +++++++----
 lib/IR/Verifier.cpp                              | 13 ++++++++-----
 test/CodeGen/X86/statepoint-call-lowering.ll     | 17 +++++++++++++++++
 5 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index a8db35c..29353a6 100644
--- a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -5726,7 +5726,7 @@ SelectionDAGBuilder::lowerInvokable(TargetLowering::CallLoweringInfo &CLI,
 }
 
 void SelectionDAGBuilder::LowerCallTo(ImmutableCallSite CS, SDValue Callee,
-                                      bool isTailCall,
+                                      bool isTailCall, bool isVarArg,
                                       MachineBasicBlock *LandingPad) {
   PointerType *PT = cast<PointerType>(CS.getCalledValue()->getType());
   FunctionType *FTy = cast<FunctionType>(PT->getElementType());
@@ -5760,7 +5760,7 @@ void SelectionDAGBuilder::LowerCallTo(ImmutableCallSite CS, SDValue Callee,
   TargetLowering::CallLoweringInfo CLI(DAG);
   CLI.setDebugLoc(getCurSDLoc()).setChain(getRoot())
     .setCallee(RetTy, FTy, Callee, std::move(Args), CS)
-    .setTailCall(isTailCall);
+    .setTailCall(isTailCall).setVarArg(isVarArg);
   std::pair<SDValue,SDValue> Result = lowerInvokable(CLI, LandingPad);
 
   if (Result.first.getNode())
diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h
index eba98b8..4724982 100644
--- a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h
+++ b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h
@@ -645,7 +645,7 @@ public:
   void CopyToExportRegsIfNeeded(const Value *V);
   void ExportFromCurrentBlock(const Value *V);
   void LowerCallTo(ImmutableCallSite CS, SDValue Callee, bool IsTailCall,
-                   MachineBasicBlock *LandingPad = nullptr);
+                   bool isVarArg = false, MachineBasicBlock *LandingPad = nullptr);
 
   std::pair<SDValue, SDValue> lowerCallOperands(
           ImmutableCallSite CS,
diff --git a/lib/CodeGen/SelectionDAG/StatepointLowering.cpp b/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
index 94f09bd..b7a582d 100644
--- a/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
+++ b/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
@@ -238,6 +238,9 @@ static SDNode *lowerCallFromStatepoint(const CallInst &CI,
   // differently.  Hopefully, this is slightly more robust w.r.t. calling
   // convention, return values, and other function attributes.
   Value *ActualCallee = const_cast<Value *>(StatepointOperands.actualCallee());
+  FunctionType *FTy =
+    cast<FunctionType>(cast<PointerType>(ActualCallee->getType())->getElementType());
+  bool isVarArg = FTy->isVarArg();
 
   std::vector<Value *> Args;
   CallInst::const_op_iterator arg_begin = StatepointOperands.call_args_begin();
@@ -249,7 +252,7 @@ static SDNode *lowerCallFromStatepoint(const CallInst &CI,
   Tmp->setTailCall(CI.isTailCall());
   Tmp->setCallingConv(CI.getCallingConv());
   Tmp->setAttributes(CI.getAttributes());
-  Builder.LowerCallTo(Tmp, Builder.getValue(ActualCallee), false);
+  Builder.LowerCallTo(Tmp, Builder.getValue(ActualCallee), false, isVarArg);
 
   // Handle the return value of the call iff any.
   const bool HasDef = !Tmp->getType()->isVoidTy();
@@ -275,10 +278,10 @@ static SDNode *lowerCallFromStatepoint(const CallInst &CI,
 
   // We just emitted a call, so it should be last thing generated
   SDValue Chain = Builder.DAG.getRoot();
-
-  // Find closest CALLSEQ_END walking back through lowered nodes if needed
   SDNode *CallEnd = Chain.getNode();
   int Sanity = 0;
+
+  // Find closest CALLSEQ_END walking back through lowered nodes if needed
   while (CallEnd->getOpcode() != ISD::CALLSEQ_END) {
     CallEnd = CallEnd->getGluedNode();
     assert(CallEnd && "Can not find call node");
@@ -289,7 +292,7 @@ static SDNode *lowerCallFromStatepoint(const CallInst &CI,
          "Expected a callseq node.");
   assert(CallEnd->getGluedNode());
 
-  // Step back inside the CALLSEQ
+  // Step back inside the CALLSEQ or VAARG
   CallNode = CallEnd->getGluedNode();
   return CallNode;
 }
diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp
index 4bf2d1a..cfd1b6c 100644
--- a/lib/IR/Verifier.cpp
+++ b/lib/IR/Verifier.cpp
@@ -2639,8 +2639,6 @@ void Verifier::visitIntrinsicFunctionCall(Intrinsic::ID ID, CallInst &CI) {
             "gc.statepoint callee must be of function pointer type",
             &CI, Target);
     FunctionType *TargetFuncType = cast<FunctionType>(PT->getElementType());
-    Assert1(!TargetFuncType->isVarArg(),
-            "gc.statepoint support for var arg functions not implemented", &CI);
 
     const Value *NumCallArgsV = CI.getArgOperand(1);
     Assert1(isa<ConstantInt>(NumCallArgsV),
@@ -2650,8 +2648,13 @@ void Verifier::visitIntrinsicFunctionCall(Intrinsic::ID ID, CallInst &CI) {
     Assert1(NumCallArgs >= 0,
             "gc.statepoint number of arguments to underlying call "
             "must be positive", &CI);
-    Assert1(NumCallArgs == (int)TargetFuncType->getNumParams(),
-            "gc.statepoint mismatch in number of call args", &CI);
+    const int NumParams = (int)TargetFuncType->getNumParams();
+    if (TargetFuncType->isVarArg())
+      Assert1(NumCallArgs >= NumParams,
+              "gc.statepoint mismatch in number of vararg call args", &CI);
+    else
+      Assert1(NumCallArgs == NumParams,
+              "gc.statepoint mismatch in number of call args", &CI);
 
     const Value *Unused = CI.getArgOperand(2);
     Assert1(isa<ConstantInt>(Unused) &&
@@ -2660,7 +2663,7 @@ void Verifier::visitIntrinsicFunctionCall(Intrinsic::ID ID, CallInst &CI) {
 
     // Verify that the types of the call parameter arguments match
     // the type of the wrapped callee.
-    for (int i = 0; i < NumCallArgs; i++) {
+    for (int i = 0; i < NumParams; i++) {
       Type *ParamType = TargetFuncType->getParamType(i);
       Type *ArgType = CI.getArgOperand(3+i)->getType();
       Assert1(ArgType == ParamType,
diff --git a/test/CodeGen/X86/statepoint-call-lowering.ll b/test/CodeGen/X86/statepoint-call-lowering.ll
index 0f1ebbe..1b696c3 100644
--- a/test/CodeGen/X86/statepoint-call-lowering.ll
+++ b/test/CodeGen/X86/statepoint-call-lowering.ll
@@ -9,6 +9,7 @@ declare zeroext i1 @return_i1()
 declare zeroext i32 @return_i32()
 declare i32* @return_i32ptr()
 declare float @return_float()
+declare i1 @varargf(i32, ...)
 
 define i1 @test_i1_return() gc "statepoint-example" {
 ; CHECK-LABEL: test_i1_return
@@ -75,6 +76,20 @@ entry:
   ret i1 %call2
 }
 
+define i1 @test_vararg(i32 addrspace(1)* %a) gc "statepoint-example" {
+; CHECK-LABEL: test_vararg
+; Check that an ununsed relocate has no code-generation impact
+; CHECK: pushq %rax
+; CHECK: callq varargf
+; CHECK-NEXT: .Ltmp16:
+; CHECK-NEXT: popq %rdx
+; CHECK-NEXT: retq
+entry:
+  %safepoint_token = tail call i32 (i1 (i32, ...)*, i32, i32, ...)* @llvm.experimental.gc.statepoint.p0f_i1i32varargf(i1 (i32, ...)* @varargf, i32 2, i32 0, i32 42, i32 43, i32 0, i32 addrspace(1)* %a)
+  %call = call zeroext i1 @llvm.experimental.gc.result.int.i1(i32 %safepoint_token)
+  ret i1 %call
+}
+
 declare i32 @llvm.experimental.gc.statepoint.p0f_i1f(i1 ()*, i32, i32, ...)
 declare i1 @llvm.experimental.gc.result.int.i1(i32)
 
@@ -87,4 +102,6 @@ declare i32* @llvm.experimental.gc.result.ptr.p0i32(i32)
 declare i32 @llvm.experimental.gc.statepoint.p0f_f32f(float ()*, i32, i32, ...)
 declare float @llvm.experimental.gc.result.float.f32(i32)
 
+declare i32 @llvm.experimental.gc.statepoint.p0f_i1i32varargf(i1 (i32, ...)*, i32, i32, ...)
+
 declare i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(i32, i32, i32)
--
2.2.1

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [INCOMPLETE] [GC] Support wrapping vararg functions in statepoint

Philip Reames-4
Your verifier changes look reasonable.  The other changes look
questionable though.  Non gc.statepoint calls to var arg functions
*definitely* work.  Any change outside of statepoint lowering is highly
suspect.

I'm pretty sure that calling a vararg function through a statepoint
already works without code change (other than the verifier).  (Or at
least simple cases work.)  Figuring out which work and which don't would
give some insight.

Philip


On 01/18/2015 08:43 PM, Ramkumar Ramachandra wrote:

> I actually need this feature quite badly in my untyped language
> compiler: since I support first-class functions, I've made the types of
> all functions a standard vararg (so I can box them).
>
> The implementation crashes when I try to read out the value of
> gc.result. Hints as to what might be wrong?
>
> Signed-off-by: Ramkumar Ramachandra <[hidden email]>
> ---
>   lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp |  4 ++--
>   lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h   |  2 +-
>   lib/CodeGen/SelectionDAG/StatepointLowering.cpp  | 11 +++++++----
>   lib/IR/Verifier.cpp                              | 13 ++++++++-----
>   test/CodeGen/X86/statepoint-call-lowering.ll     | 17 +++++++++++++++++
>   5 files changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> index a8db35c..29353a6 100644
> --- a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> +++ b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> @@ -5726,7 +5726,7 @@ SelectionDAGBuilder::lowerInvokable(TargetLowering::CallLoweringInfo &CLI,
>   }
>  
>   void SelectionDAGBuilder::LowerCallTo(ImmutableCallSite CS, SDValue Callee,
> -                                      bool isTailCall,
> +                                      bool isTailCall, bool isVarArg,
>                                         MachineBasicBlock *LandingPad) {
>     PointerType *PT = cast<PointerType>(CS.getCalledValue()->getType());
>     FunctionType *FTy = cast<FunctionType>(PT->getElementType());
> @@ -5760,7 +5760,7 @@ void SelectionDAGBuilder::LowerCallTo(ImmutableCallSite CS, SDValue Callee,
>     TargetLowering::CallLoweringInfo CLI(DAG);
>     CLI.setDebugLoc(getCurSDLoc()).setChain(getRoot())
>       .setCallee(RetTy, FTy, Callee, std::move(Args), CS)
> -    .setTailCall(isTailCall);
> +    .setTailCall(isTailCall).setVarArg(isVarArg);
>     std::pair<SDValue,SDValue> Result = lowerInvokable(CLI, LandingPad);
>  
>     if (Result.first.getNode())
> diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h
> index eba98b8..4724982 100644
> --- a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h
> +++ b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h
> @@ -645,7 +645,7 @@ public:
>     void CopyToExportRegsIfNeeded(const Value *V);
>     void ExportFromCurrentBlock(const Value *V);
>     void LowerCallTo(ImmutableCallSite CS, SDValue Callee, bool IsTailCall,
> -                   MachineBasicBlock *LandingPad = nullptr);
> +                   bool isVarArg = false, MachineBasicBlock *LandingPad = nullptr);
>  
>     std::pair<SDValue, SDValue> lowerCallOperands(
>             ImmutableCallSite CS,
> diff --git a/lib/CodeGen/SelectionDAG/StatepointLowering.cpp b/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
> index 94f09bd..b7a582d 100644
> --- a/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
> +++ b/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
> @@ -238,6 +238,9 @@ static SDNode *lowerCallFromStatepoint(const CallInst &CI,
>     // differently.  Hopefully, this is slightly more robust w.r.t. calling
>     // convention, return values, and other function attributes.
>     Value *ActualCallee = const_cast<Value *>(StatepointOperands.actualCallee());
> +  FunctionType *FTy =
> +    cast<FunctionType>(cast<PointerType>(ActualCallee->getType())->getElementType());
> +  bool isVarArg = FTy->isVarArg();
>  
>     std::vector<Value *> Args;
>     CallInst::const_op_iterator arg_begin = StatepointOperands.call_args_begin();
> @@ -249,7 +252,7 @@ static SDNode *lowerCallFromStatepoint(const CallInst &CI,
>     Tmp->setTailCall(CI.isTailCall());
>     Tmp->setCallingConv(CI.getCallingConv());
>     Tmp->setAttributes(CI.getAttributes());
> -  Builder.LowerCallTo(Tmp, Builder.getValue(ActualCallee), false);
> +  Builder.LowerCallTo(Tmp, Builder.getValue(ActualCallee), false, isVarArg);
>  
>     // Handle the return value of the call iff any.
>     const bool HasDef = !Tmp->getType()->isVoidTy();
> @@ -275,10 +278,10 @@ static SDNode *lowerCallFromStatepoint(const CallInst &CI,
>  
>     // We just emitted a call, so it should be last thing generated
>     SDValue Chain = Builder.DAG.getRoot();
> -
> -  // Find closest CALLSEQ_END walking back through lowered nodes if needed
>     SDNode *CallEnd = Chain.getNode();
>     int Sanity = 0;
> +
> +  // Find closest CALLSEQ_END walking back through lowered nodes if needed
>     while (CallEnd->getOpcode() != ISD::CALLSEQ_END) {
>       CallEnd = CallEnd->getGluedNode();
>       assert(CallEnd && "Can not find call node");
> @@ -289,7 +292,7 @@ static SDNode *lowerCallFromStatepoint(const CallInst &CI,
>            "Expected a callseq node.");
>     assert(CallEnd->getGluedNode());
>  
> -  // Step back inside the CALLSEQ
> +  // Step back inside the CALLSEQ or VAARG
>     CallNode = CallEnd->getGluedNode();
>     return CallNode;
>   }
> diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp
> index 4bf2d1a..cfd1b6c 100644
> --- a/lib/IR/Verifier.cpp
> +++ b/lib/IR/Verifier.cpp
> @@ -2639,8 +2639,6 @@ void Verifier::visitIntrinsicFunctionCall(Intrinsic::ID ID, CallInst &CI) {
>               "gc.statepoint callee must be of function pointer type",
>               &CI, Target);
>       FunctionType *TargetFuncType = cast<FunctionType>(PT->getElementType());
> -    Assert1(!TargetFuncType->isVarArg(),
> -            "gc.statepoint support for var arg functions not implemented", &CI);
>  
>       const Value *NumCallArgsV = CI.getArgOperand(1);
>       Assert1(isa<ConstantInt>(NumCallArgsV),
> @@ -2650,8 +2648,13 @@ void Verifier::visitIntrinsicFunctionCall(Intrinsic::ID ID, CallInst &CI) {
>       Assert1(NumCallArgs >= 0,
>               "gc.statepoint number of arguments to underlying call "
>               "must be positive", &CI);
> -    Assert1(NumCallArgs == (int)TargetFuncType->getNumParams(),
> -            "gc.statepoint mismatch in number of call args", &CI);
> +    const int NumParams = (int)TargetFuncType->getNumParams();
> +    if (TargetFuncType->isVarArg())
> +      Assert1(NumCallArgs >= NumParams,
> +              "gc.statepoint mismatch in number of vararg call args", &CI);
> +    else
> +      Assert1(NumCallArgs == NumParams,
> +              "gc.statepoint mismatch in number of call args", &CI);
>  
>       const Value *Unused = CI.getArgOperand(2);
>       Assert1(isa<ConstantInt>(Unused) &&
> @@ -2660,7 +2663,7 @@ void Verifier::visitIntrinsicFunctionCall(Intrinsic::ID ID, CallInst &CI) {
>  
>       // Verify that the types of the call parameter arguments match
>       // the type of the wrapped callee.
> -    for (int i = 0; i < NumCallArgs; i++) {
> +    for (int i = 0; i < NumParams; i++) {
>         Type *ParamType = TargetFuncType->getParamType(i);
>         Type *ArgType = CI.getArgOperand(3+i)->getType();
>         Assert1(ArgType == ParamType,
> diff --git a/test/CodeGen/X86/statepoint-call-lowering.ll b/test/CodeGen/X86/statepoint-call-lowering.ll
> index 0f1ebbe..1b696c3 100644
> --- a/test/CodeGen/X86/statepoint-call-lowering.ll
> +++ b/test/CodeGen/X86/statepoint-call-lowering.ll
> @@ -9,6 +9,7 @@ declare zeroext i1 @return_i1()
>   declare zeroext i32 @return_i32()
>   declare i32* @return_i32ptr()
>   declare float @return_float()
> +declare i1 @varargf(i32, ...)
>  
>   define i1 @test_i1_return() gc "statepoint-example" {
>   ; CHECK-LABEL: test_i1_return
> @@ -75,6 +76,20 @@ entry:
>     ret i1 %call2
>   }
>  
> +define i1 @test_vararg(i32 addrspace(1)* %a) gc "statepoint-example" {
> +; CHECK-LABEL: test_vararg
> +; Check that an ununsed relocate has no code-generation impact
> +; CHECK: pushq %rax
> +; CHECK: callq varargf
> +; CHECK-NEXT: .Ltmp16:
> +; CHECK-NEXT: popq %rdx
> +; CHECK-NEXT: retq
> +entry:
> +  %safepoint_token = tail call i32 (i1 (i32, ...)*, i32, i32, ...)* @llvm.experimental.gc.statepoint.p0f_i1i32varargf(i1 (i32, ...)* @varargf, i32 2, i32 0, i32 42, i32 43, i32 0, i32 addrspace(1)* %a)
> +  %call = call zeroext i1 @llvm.experimental.gc.result.int.i1(i32 %safepoint_token)
> +  ret i1 %call
> +}
> +
>   declare i32 @llvm.experimental.gc.statepoint.p0f_i1f(i1 ()*, i32, i32, ...)
>   declare i1 @llvm.experimental.gc.result.int.i1(i32)
>  
> @@ -87,4 +102,6 @@ declare i32* @llvm.experimental.gc.result.ptr.p0i32(i32)
>   declare i32 @llvm.experimental.gc.statepoint.p0f_f32f(float ()*, i32, i32, ...)
>   declare float @llvm.experimental.gc.result.float.f32(i32)
>  
> +declare i32 @llvm.experimental.gc.statepoint.p0f_i1i32varargf(i1 (i32, ...)*, i32, i32, ...)
> +
>   declare i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(i32, i32, i32)

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [INCOMPLETE] [GC] Support wrapping vararg functions in statepoint

Ramkumar Ramachandra
Philip Reames wrote:
> Any change outside of statepoint lowering is highly suspect.

Notice that SelectionDAGBuilder::LowerCallTo (the one I'm modifying)
has exactly one other caller: visitCall, which doesn't match vararg
functions. Every other codepath directly calls
TargetLowering::LowerCallTo, supplying CallLoweringInfo information
explicity (it's a structure with a vararg field).

I suspect I'm not overloading SelectionDAGBuilder enough.

> I'm pretty sure that calling a vararg function through a statepoint already
> works without code change (other than the verifier).  (Or at least simple
> cases work.)  Figuring out which work and which don't would give some
> insight.

AFAICT, extracting the result from a statepoint wrapping a vararg
function is the problem. If it's a void function, there's no problem.
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [INCOMPLETE] [GC] Support wrapping vararg functions in statepoint

Philip Reames-4
On 01/20/2015 10:11 AM, Ramkumar Ramachandra wrote:
> Philip Reames wrote:
>> Any change outside of statepoint lowering is highly suspect.
> Notice that SelectionDAGBuilder::LowerCallTo (the one I'm modifying)
> has exactly one other caller: visitCall, which doesn't match vararg
> functions. Every other codepath directly calls
> TargetLowering::LowerCallTo, supplying CallLoweringInfo information
> explicity (it's a structure with a vararg field).
I think I'm missing something here.  What leads you to believe that
visitCall doesn't handle vararg functions?

Also, I see calls in both visitInvoke and visitCall.  I would assume
these are the primary entry points...

>
> I suspect I'm not overloading SelectionDAGBuilder enough.
>
>> I'm pretty sure that calling a vararg function through a statepoint already
>> works without code change (other than the verifier).  (Or at least simple
>> cases work.)  Figuring out which work and which don't would give some
>> insight.
> AFAICT, extracting the result from a statepoint wrapping a vararg
> function is the problem. If it's a void function, there's no problem.
Ah!  That helps to understand the problem.

Can you separate out a change to the verifier which accepts void
returning varargs and post that for review?  I can sign off quickly.

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [INCOMPLETE] [GC] Support wrapping vararg functions in statepoint

Ramkumar Ramachandra
Philip Reames wrote:
> Can you separate out a change to the verifier which accepts void returning
> varargs and post that for review?

Please double-check that it works on your end.
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [INCOMPLETE] [GC] Support wrapping vararg functions in statepoint

Ramkumar Ramachandra
In reply to this post by Philip Reames-4
Philip Reames wrote:
> I think I'm missing something here.  What leads you to believe that
> visitCall doesn't handle vararg functions?
>
> Also, I see calls in both visitInvoke and visitCall.  I would assume these
> are the primary entry points...

I'm probably more confused about this than you are.

The callchain is approximately SelectionDAGBuilder::LowerCallTo ->
SelectionDAGBuilder::lowerInvokable -> TargetLowering::LowerCallTo ->
X86TargetLowering::LowerCall. Now, the isVarArg information needs to
be conveyed to LowerCall, and that happens via CLI.IsVarArg, which
TargetLowering::LowerCallTo accepts as a parameter.
SelectionDAGBuilder::LowerCallTo isn't passed that information, and
doesn't compute it to send to lowerInvokable (which builds the CLI for
TargetLowering::LowerCallTo).

How could SelectionDAGBuilder::LowerCallTo possibly handle vararg?
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: [INCOMPLETE] [GC] Support wrapping vararg functions in statepoint

Philip Reames-4
On 01/20/2015 11:28 AM, Ramkumar Ramachandra wrote:

> Philip Reames wrote:
>> I think I'm missing something here.  What leads you to believe that
>> visitCall doesn't handle vararg functions?
>>
>> Also, I see calls in both visitInvoke and visitCall.  I would assume these
>> are the primary entry points...
> I'm probably more confused about this than you are.
>
> The callchain is approximately SelectionDAGBuilder::LowerCallTo ->
> SelectionDAGBuilder::lowerInvokable -> TargetLowering::LowerCallTo ->
> X86TargetLowering::LowerCall. Now, the isVarArg information needs to
> be conveyed to LowerCall, and that happens via CLI.IsVarArg, which
> TargetLowering::LowerCallTo accepts as a parameter.
> SelectionDAGBuilder::LowerCallTo isn't passed that information, and
> doesn't compute it to send to lowerInvokable (which builds the CLI for
> TargetLowering::LowerCallTo).
>
> How could SelectionDAGBuilder::LowerCallTo possibly handle vararg?
Step 1 - you should write a test case and check

However, I'd expect - which may be completely wrong! - that the 'vararg
ness' of the call is either a property of the actually callee, the
callsite, or both.  Given we do pass the callee into CLI, it *might*
happen that way.  In fact, looking at CLI::setCallee, this is exactly
what I see.
http://llvm.org/docs/doxygen/html/TargetLowering_8h_source.html#l02347

Philip

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev