[PATCH] D100009: [Statepoint Lowering] Allow other than N byte sized types in deopt bundle

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp:62
+// TODO: This really should be a property of the callers calling convention.
+static cl::opt<unsigned> PromoteSlotBitsInSelectionDAG(
+    "promote-slot-bits-in-selection-dag", cl::Hidden, cl::init(0),
----------------
I don't think a command line argument is the right way to describe this.  I strongly suspect that this number is a property of the calling convention of the call (or maybe a combination of the callers calling convention and the calls calling convention.)   If so, I would suggest reframing it in that manner.

For calling conventions which don't explicitly override it to something wider, using the abi alignment (e.g. the same we'd use for argument passing) seems like a reasonable default.  That should also give you way to write tests without upstreaming the custom calling convention motivating this.  


================
Comment at: llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp:179
+  EVT VTy = Incoming.getValueType();
+  if (PromoteSlotBitsInSelectionDAG &&
+      VTy.getSizeInBits() < PromoteSlotBitsInSelectionDAG) {
----------------
Style: Please use early return.  


================
Comment at: llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp:413
   if (!Loc.getNode()) {
-    Loc = Builder.StatepointLowering.allocateStackSlot(Incoming.getValueType(),
-                                                       Builder);
+    EVT VTy = Incoming.getValueType();
+    if (PromoteSlotBitsInSelectionDAG &&
----------------
A suggestion.  

If you split the promote function into one which produced the promoted type, and one which did the actual promotion, I think you can simplify this code, make it easier to keep in sync, and probably simplify some of your asserts.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp:431
+
+#ifndef NDEBUG
+    int64_t ExpectedSlotSize = (int64_t)Incoming.getValueSizeInBits();
----------------
In particular, previous suggestion should greatly simplify this block of code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100009