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

Thu Mar 12 14:07:09 PDT 2020

efriedma added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:3056
+    StackID = TargetStackID::SVEVector;
   }
   assert(Opc && "Unknown register class");
----------------
c-rhodes wrote:
> 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
I think my concern was just that it's not clear what the return value of getSpillSize() means, if it's not the size in bytes of the required spill slot.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75988