[PATCH] D75988: [AArch64][SVE] Add support for spilling/filling ZPR2/3/4

Thu Mar 12 03:46:55 PDT 2020

c-rhodes added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:3056
+    StackID = TargetStackID::SVEVector;
   }
   assert(Opc && "Unknown register class");
----------------
cameron.mcinally wrote:
> Nit: Should these be the None/default case of the switch above?
> 
> Seems wasteful to keep searching if we've already found our opcode in the switch.
That's seems like a sensible suggestion, it would be nicer if this was handled in the existing switch. I noticed a comment from @efriedma [1] on Sander's patch adding the same functionality for ZPR/PPR classes,  where he mentions not depending on what `getSpillSize` returns. @efriedma do you recall why this was?

I've had a brief chat with Sander about this and as far as I'm aware we haven't formally decided on how the spill size is represented for scalable vectors, currently it's 128 for ZPR register class and 16 for PPR with an implicit vscale multiplier. Perhaps these could be moved to the relevant cases in that switch, i.e. `2` for PPR, `48` for ZPR3 etc.

[1] https://reviews.llvm.org/D70082?id=228687#inline-631144

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75988