Skip to content

Commit

Permalink
[ARM] Fix insert point for store rescheduling.
Browse files Browse the repository at this point in the history
    
In ARMPreAllocLoadStoreOpt::RescheduleOps, LastOp should be the last
operation which we want to merge. If we break out of the loop because
an operation has the wrong offset, we shouldn't use that operation as
LastOp.
    
This patch fixes some cases where we would sink stores for no reason.
    
Differential Revision: https://reviews.llvm.org/D30124

llvm-svn: 296708
  • Loading branch information
Eli Friedman committed Mar 1, 2017
1 parent 923a320 commit 1c9216b
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 25 deletions.
30 changes: 18 additions & 12 deletions llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp
Expand Up @@ -2161,33 +2161,39 @@ bool ARMPreAllocLoadStoreOpt::RescheduleOps(MachineBasicBlock *MBB,
unsigned LastBytes = 0;
unsigned NumMove = 0;
for (int i = Ops.size() - 1; i >= 0; --i) {
// Make sure each operation has the same kind.
MachineInstr *Op = Ops[i];
unsigned Loc = MI2LocMap[Op];
if (Loc <= FirstLoc) {
FirstLoc = Loc;
FirstOp = Op;
}
if (Loc >= LastLoc) {
LastLoc = Loc;
LastOp = Op;
}

unsigned LSMOpcode
= getLoadStoreMultipleOpcode(Op->getOpcode(), ARM_AM::ia);
if (LastOpcode && LSMOpcode != LastOpcode)
break;

// Check that we have a continuous set of offsets.
int Offset = getMemoryOpOffset(*Op);
unsigned Bytes = getLSMultipleTransferSize(Op);
if (LastBytes) {
if (Bytes != LastBytes || Offset != (LastOffset + (int)Bytes))
break;
}

// Don't try to reschedule too many instructions.
if (++NumMove == 8) // FIXME: Tune this limit.
break;

// Found a mergable instruction; save information about it.
LastOffset = Offset;
LastBytes = Bytes;
LastOpcode = LSMOpcode;
if (++NumMove == 8) // FIXME: Tune this limit.
break;

unsigned Loc = MI2LocMap[Op];
if (Loc <= FirstLoc) {
FirstLoc = Loc;
FirstOp = Op;
}
if (Loc >= LastLoc) {
LastLoc = Loc;
LastOp = Op;
}
}

if (NumMove <= 1)
Expand Down
32 changes: 19 additions & 13 deletions llvm/test/CodeGen/ARM/prera-ldst-insertpt.mir
Expand Up @@ -36,19 +36,22 @@ body: |
t2STRi12 %1, %0, 0, 14, _ :: (store 4)
%10 : rgpr = t2LSLri %2, 1, 14, _, _
t2STRi12 killed %10, %0, 4, 14, _ :: (store 4)
; Make sure we move the paired stores next to each other, and
; insert them in an appropriate location.
; CHECK: t2STRi12 %1,
; CHECK-NEXT: t2STRi12 killed %10,
; CHECK-NEXT: t2MOVi
; CHECK-NEXT: t2ADDrs
%11 : rgpr = t2MOVi 55, 14, _, _
%12 : gprnopc = t2ADDrs %11, killed %7, 19, 14, _, _
t2STRi12 killed %12, %0, 16, 14, _ :: (store 4)
%13 : gprnopc = t2ADDrs %11, killed %9, 19, 14, _, _
t2STRi12 killed %13, %0, 20, 14, _ :: (store 4)
; Make sure we move the paired stores next to each other.
; FIXME: Make sure we don't extend the live-range of a store
; when we don't need to.
; CHECK: t2STRi12 %1,
; CHECK-NEXT: t2STRi12 killed %10,
; CHECK-NEXT: %13 = t2ADDrs %11
; CHECK-NEXT: t2STRi12 killed %12,
; CHECK: t2STRi12 killed %12,
; CHECK-NEXT: t2STRi12 killed %13,
tBX_RET 14, _
Expand All @@ -73,6 +76,15 @@ body: |
t2STRi12 killed %10, %0, 4, 14, _ :: (store 4)
%3 : rgpr = t2MUL %2, %2, 14, _
t2STRi12 %3, %0, 8, 14, _ :: (store 4)
; Make sure we move the paired stores next to each other, and
; insert them in an appropriate location.
; CHECK: t2STRi12 {{.*}}, 0
; CHECK-NEXT: t2STRi12 {{.*}}, 4
; CHECK-NEXT: t2STRi12 {{.*}}, 8
; CHECK-NEXT: t2MUL
; CHECK-NEXT: t2MOVi32imm
%4 : rgpr = t2MUL %1, %1, 14, _
%5 : rgpr = t2MOVi32imm -858993459
%6 : rgpr, %7 : rgpr = t2UMULL killed %3, %5, 14, _
Expand All @@ -85,13 +97,7 @@ body: |
t2STRi12 killed %13, %0, 20, 14, _ :: (store 4)
; Make sure we move the paired stores next to each other.
; FIXME: Make sure we don't extend the live-range of a store
; when we don't need to.
; CHECK: t2STRi12 {{.*}}, 0
; CHECK-NEXT: t2STRi12 {{.*}}, 4
; CHECK-NEXT: t2STRi12 {{.*}}, 8
; CHECK-NEXT: t2ADDrs
; CHECK-NEXT: t2STRi12 {{.*}}, 16
; CHECK: t2STRi12 {{.*}}, 16
; CHECK-NEXT: t2STRi12 {{.*}}, 20
tBX_RET 14, _
Expand Down

0 comments on commit 1c9216b

Please sign in to comment.