Skip to content

Commit

Permalink
[AArch64] Fix offset calculation
Browse files Browse the repository at this point in the history
r374772 changed Offset to be an int64_t but left NewOffset as an int.
Scale is unsigned, so in the calculation `Offset - NewOffset * Scale`,
`NewOffset * Scale` was promoted to unsigned and was then zero-extended
to 64 bits, leading to an incorrect computation which manifested as an
out-of-memory when building the Swift standard library for Android
aarch64. Promote NewOffset to int64_t to fix this, and promote
EmittableOffset as well, since its one user passes it to a function
which takes an int64_t anyway.

Test case based on a suggestion by Sander de Smalen!

Differential Revision: https://reviews.llvm.org/D69018

llvm-svn: 375043
  • Loading branch information
smeenai committed Oct 16, 2019
1 parent 34ed76e commit 6d1891c
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 4 deletions.
6 changes: 3 additions & 3 deletions llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
Expand Up @@ -3368,7 +3368,7 @@ int llvm::isAArch64FrameOffsetLegal(const MachineInstr &MI,
StackOffset &SOffset,
bool *OutUseUnscaledOp,
unsigned *OutUnscaledOp,
int *EmittableOffset) {
int64_t *EmittableOffset) {
// Set output values in case of early exit.
if (EmittableOffset)
*EmittableOffset = 0;
Expand Down Expand Up @@ -3430,7 +3430,7 @@ int llvm::isAArch64FrameOffsetLegal(const MachineInstr &MI,
"Cannot have remainder when using unscaled op");

assert(MinOff < MaxOff && "Unexpected Min/Max offsets");
int NewOffset = Offset / Scale;
int64_t NewOffset = Offset / Scale;
if (MinOff <= NewOffset && NewOffset <= MaxOff)
Offset = Remainder;
else {
Expand Down Expand Up @@ -3471,7 +3471,7 @@ bool llvm::rewriteAArch64FrameIndex(MachineInstr &MI, unsigned FrameRegIdx,
return true;
}

int NewOffset;
int64_t NewOffset;
unsigned UnscaledOp;
bool UseUnscaledOp;
int Status = isAArch64FrameOffsetLegal(MI, Offset, &UseUnscaledOp,
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/AArch64/AArch64InstrInfo.h
Expand Up @@ -335,7 +335,7 @@ enum AArch64FrameOffsetStatus {
int isAArch64FrameOffsetLegal(const MachineInstr &MI, StackOffset &Offset,
bool *OutUseUnscaledOp = nullptr,
unsigned *OutUnscaledOp = nullptr,
int *EmittableOffset = nullptr);
int64_t *EmittableOffset = nullptr);

static inline bool isUncondBranchOpcode(int Opc) { return Opc == AArch64::B; }

Expand Down
17 changes: 17 additions & 0 deletions llvm/test/CodeGen/AArch64/framelayout-offset-immediate-change.mir
@@ -0,0 +1,17 @@
# RUN: llc -mtriple=aarch64-none-linux-gnu -run-pass=prologepilog %s -o - | FileCheck %s
---
name: framelayout_offset_immediate_change
tracksRegLiveness: true
fixedStack:
- { id: 0, offset: 0, size: 1}
body: |
bb.0:
$x0 = LDURXi %fixed-stack.0, -264
RET_ReallyLR
...
# CHECK: name: framelayout_offset_immediate_change
# CHECK: body: |
# CHECK-NEXT: bb.0:
# CHECK-NEXT: $x8 = SUBXri $sp, 8, 0
# CHECK-NEXT: $x0 = LDURXi killed $x8, -256
# CHECK-NEXT: RET_ReallyLR

0 comments on commit 6d1891c

Please sign in to comment.