[PATCH] D61436: [AArch64] NFC: Generalize emitFrameOffset to support more than byte offsets.

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 3 16:46:49 PDT 2019


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

LGTM



================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:3045
+  // First emit non-scalable frame offsets, or a simple 'mov'.
+  if (Bytes || (Offset.isZero() && SrcReg != DestReg)) {
+    assert((DestReg != AArch64::SP || Bytes % 16 == 0) &&
----------------
sdesmalen wrote:
> efriedma wrote:
> > What is Offset.isZero() supposed to be checking here?
> For the case where the StackOffset can contain both (fixed-size) bytes, and scalable bytes (i.e. MVT::nxv1i8 for SVE where we know there are 'n x bytes', with 'n' being a runtime constant), we want to use ADDXri to emit a frame-offset if:
> - Bytes are non-zero (which would mean a meaningful "add" for the fixed-size part of the stack offset)
> Or:
> - To copy the value of SrcReg to DestReg, but only if StackOffset is empty in its entirety (checking that Bytes are non-zero is not enough).
> 
> The functionality is added in {D61437} (lines 3067-3082).
Oh, you don't want to emit the mov if the offset isn't empty because it would be redundant.  That makes sense.


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

https://reviews.llvm.org/D61436





More information about the llvm-commits mailing list