[PATCH] D131582: [AArch64][SME] Add support for arm_locally_streaming functions.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp:1162
   case ISD::EntryToken:
-    llvm_unreachable("EntryToken should have been excluded from the schedule!");
   case ISD::MERGE_VALUES:
----------------
aemerson wrote:
> ?
Perhaps removing this without any alternative `assert` is a little crude, but I wasn't really sure what to test for instead.

The change here is that EntryToken now "becomes part of the schedule" because other instructions can be glued to it, even though the node itself isn't schedulable (it is always guaranteed to be the first instruction).  I was hoping someone would have a suggestion here.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:1239
+  // passing a getVTList(MVT::Other, MVT::Glue) in its definition above led to a
+  // SegFault when trying to add the VTList.
+  EntryNode =
----------------
aemerson wrote:
> I think we need a better understanding.
You're right. The issue was that calling `getVTList` with more than one operand went through a different code-path which queries the `VTListMap`, which is initialised //after// EntryNode. By reordering these member variables (lexicographically) the issue goes away.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131582