[PATCH] Lower invoke statepoints and gc.results tied to them.

Igor Laevsky igor at azulsystems.com
Thu Feb 26 14:13:56 PST 2015


In http://reviews.llvm.org/D7760#126890, @reames wrote:

> Comments inline.



REPOSITORY
  rL LLVM

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:257
@@ +256,3 @@
+      // so we need to export it now.
+      unsigned reg = Builder.FuncInfo.CreateRegs(Tmp->getType());
+      Builder.CopyValueToVirtualRegister(Tmp, reg);
----------------
reames wrote:
> I'm still confused about why we need this.  Why doesn't the standard export mechanism work here?  Simply producing the value and letting it be exported normally seems like it would work?
Main problem here is that value we want to export is 'Tmp' which will be deleted few lines later. Therefore we need to export statepoint call itself, but it has different type than the actual call. Default mechanism for exporting values from BasicBlock calculates type of the register from Value* we are trying to export. So we need to create register with correct type manually. 

This probably could be wrapped in a helper function or an overload of a 'ExportFromCurrentBlock(Value*)' but it's the only place such problem appears so probably not worth it.

I have added comment describing this.

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:297
@@ -277,2 +296,3 @@
   int Sanity = 0;
   while (CallEnd->getOpcode() != ISD::CALLSEQ_END) {
+    assert(CallEnd->getNumOperands() >= 1 &&
----------------
reames wrote:
> Just to be clear, the reason for this is that CallEnd might start with the eh_label which doesn't have a glue node?  
Yes. DAG root will be eh_label in case of invoke and it will not have glued nodes.

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:669
@@ +668,3 @@
+    // and getValue() will use CopyFromReg of the wrong type (always i32 in our case)
+    PointerType *calleeType = cast<PointerType>(
+                                ImmutableStatepoint(I).actualCallee()->getType());
----------------
reames wrote:
> Doesn't this depend on what SDValue we set for a specific instruction?   As above, I'm not convinced this is necessary.
> 
> Also, CamelCase naming per llvm docs.
SDValue is live only during processing of a single BasicBlock. We are exporting llvm-ir Value's. And in our case statepoint value has different type than the actual lowered call. This is same problem as in lowerCallFromStatepoint.

http://reviews.llvm.org/D7760

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list