[PATCH] D69018: [AArch64] Fix offset calculation

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 16 10:47:26 PDT 2019


sdesmalen added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/framelayout-large-offset.mir:14
+# CHECK:              sub     x8, sp, #4095, lsl #12
+# CHECK-NEXT:         sub     x8, x8, #4095, lsl #12
+# CHECK-NEXT:         sub     x8, x8, #4095, lsl #12
----------------
smeenai wrote:
> smeenai wrote:
> > sdesmalen wrote:
> > > Sorry I didn't notice this earlier when I posted the example MIR, but these `sub`s are wrong. The offset is at a positive distance from the SP, so it should use `add` here. If I change the offset from `2147483648` to `2147483632` that changes. So I expect some other changes are needed to fix this.
> > If I revert my change and r374772, the problem still persists, so that seems like a pre-existing issue?
> Given that this is a pre-existing problem, would you be okay with submitting this to solve the OOM and then filing a bug for the incorrect offset calculation?
Sigh, I'm clearly having a bit of a senior moment here :) maxint is 2147483647 (forgot the -1 in my calculator). But with the fixed/updated offset, the result is the same with/without your patch, so we'll need something different to test it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69018





More information about the llvm-commits mailing list