[PATCH] D80598: Start migrating away from statepoint's inline length prefixed argument bundles

Wed May 27 09:44:12 PDT 2020

reames marked an inline comment as done.
reames added a comment.

In D80598#2056404 <https://reviews.llvm.org/D80598#2056404>, @anna wrote:

> LGTM. thanks for working on this! Apart from dropping all of the extra handling code in RS4GC and verifier (which itself is a really good reason for this support btw), I'm assuming this for optimizations which know about the deopt_op bundle (or gc_transition bundle) after relocations are made explicit? i.e., a general robustness patch.

I can't quite parse what you meant with that last sentence.

The current plan is to use operand bundles throughout.  We'll still have statepoints to wrap the actual call, but all of the special operands will stay in bundles.

I did find one minor annoyance.  We apparently treat deopt bundles differently than statepoints with deopt args in the inliner and that's causing a crash with the follow up patch.  I'm going to have to restructure things a bit for the next step to be NFC.  (All in reference to patch not yet posted for review.)

================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2778
   assert(!I.hasOperandBundlesOtherThan({LLVMContext::OB_deopt,
+                                        LLVMContext::OB_gc_transition,
                                         LLVMContext::OB_funclet,
----------------
anna wrote:
> hmm. so we currently don't actually have any optimizations leveraging OB_gc_transition.
Yes, though this line is about supporting invokable statepoint points with transition bundles.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80598/new/

https://reviews.llvm.org/D80598