Ramkumar Ramachandra
2015-Jan-19 04:43 UTC
[LLVMdev] [INCOMPLETE] [GC] Support wrapping vararg functions in statepoint
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 <artagnon at gmail.com>
---
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
Philip Reames
2015-Jan-20 01:06 UTC
[LLVMdev] [INCOMPLETE] [GC] Support wrapping vararg functions in statepoint
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 <artagnon at gmail.com> > --- > 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)
Ramkumar Ramachandra
2015-Jan-20 18:11 UTC
[LLVMdev] [INCOMPLETE] [GC] Support wrapping vararg functions in statepoint
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.