[PATCH] D59635: [AArch64] NFC: Cleanup isAArch64FrameOffsetLegal

Mon Mar 25 14:00:10 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:3269
+    NewOffset = NewOffset < 0 ? MinOff : MaxOff;
+    Offset = Offset - NewOffset * Scale + Remainder;
   }
----------------
sdesmalen wrote:
> efriedma wrote:
> > It's still a little strange to compute "Offset" here, then only use the computed value to check whether it's equal to zero.  A separate variable probably makes more sense.
> Note that Offset is passed by reference, so is used as an output variable as well.
> isAArch64FrameOffsetLegal returns whether the immediate can be used to handle the offset, either in full or partially. In the latter case where the instruction can't fully handle the offset using the immediate, the remaining offset (in bytes) that needs to be handled differently is again stored in Offset.
Oh, right... it's fine then, I guess.  (Maybe could use a few more comments, but I'm not sure where, exactly.)

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

https://reviews.llvm.org/D59635