[PATCH] D18257: Add "first class" lowering for deopt operand bundles

[Sanjoy Das via llvm-commits]

sanjoy added a subscriber: rnk.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:6114
@@ +6113,3 @@
+#ifndef NDEBUG
+  for (unsigned i = 0, e = I.getNumOperandBundles(); i != e; ++i)
+    assert((I.getOperandBundleAt(i).isDeoptOperandBundle() ||
----------------
reames wrote:
> It currently looks like we're actually supporting *either* deopt operands XOR funclet operands.  I might remove the loop and just assert that nothing other than deopt operands are available on the deopt path.  
> 
> Same in the other place this loop exist.
IIUC we don't need to do anything to lower the `funclet` operand bundle, so we're trivially lowering both.  @majnemer, @rnk -- can you one of you please point out what the right behavior is here?

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:844
@@ +843,3 @@
+  StatepointLoweringInfo SI(DAG);
+  populateCallLoweringInfo(SI.CLI, CS, 0, CS.getNumArgOperands(), Callee,
+                           CS.getType(), false);
----------------
reames wrote:
> 0 as a magic value is unclear.  At least pull out a named constant CallSiteArgOperandBegin.
Changed it to `CS.arg_begin() - CS.op_begin()`

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:857
@@ +856,3 @@
+
+  StatepointDirectives SD =
+      parseStatepointDirectivesFromAttrs(CS.getAttributes());
----------------
reames wrote:
> Minor code structure: I'd move the default init immediately before each conditional init to make the value flow obvious.
Changed to use `getValueOr`.


http://reviews.llvm.org/D18257