[PATCH] D70082: [AArch64] Extend storeRegToStackSlot to spill SVE registers.

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 12 11:51:17 PST 2019


efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:2960
   bool Offset = true;
   switch (TRI->getSpillSize(*RC)) {
   case 1:
----------------
sdesmalen wrote:
> efriedma wrote:
> > I'd prefer to refactor the code so it doesn't depend on what getSpillSize returns for SVE types.
> The SVE case is actually outside the switch statement and is therefore independent of what getSpillSize returns. The switch statement seems to be there purely for having a quicker look-up as opposed to having a long `if-elseif-elseif-[..]-elseif` chain. Do you want me to change that?
Oh, I didn't read the code in the switch that carefully.  I guess it only triggers for specific register classes, so it doesn't matter what getSpillSize() returns as long as it doesn't crash.

I guess that's okay.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:3048
+    StackID = TargetStackID::SVEVector;
+  } else if (AArch64::ZPRRegClass.hasSubClassEq(RC)) {
+    assert(Subtarget.hasSVE() && "Unexpected register load without SVE");
----------------
We'll eventually need to handle ZPR2 etc.?  I guess we don't need that immediately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70082





More information about the llvm-commits mailing list