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

[Eli Friedman via Phabricator via llvm-commits]

efriedma 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:
----------------
sdesmalen wrote:
> sdesmalen wrote:
> > 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.
> @efriedma just wanting a second opinion on this, are you aware of anything else I might need to worry about here?
> 
> I needed to remove this llvm_unreachable because I changed EntryToken to also have Glue, and when using that it makes the EntryToken part of the schedule (even though the node itself can't be scheduled).
This feels pretty isolated; the entry token basically has the same meaning during isel, and we're throwing it away after isel.  The only possible side-effect I can think of is that maybe there's something assumes the entry token only has one successor.

That said, it feels like a sort of strange thing to be inserting smstart/smend this early... would it make sense to insert it as part of prologue/epilogue lowering instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131582