Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
AArch64: Remove reversedInstructionsWithoutDebug helper
When using reversedInstructionsWithoutDebug to construct a range from a pair of MachineInstrBundleIterators, the range unexpectedly leaves out an element. This results in mis-optimization as @mstorsjo points out in https://reviews.llvm.org/D78157. The problem is that when we convert a MachineInstrBundleIterator to a reverse iterator, the result gets incremented: MachineInstrBundleIterator(++I.getReverse()) The comment there explains that the "resulting iterator will dereference ... to the previous node, which is somewhat unexpected; but converting the two endpoints in a range will give the same range in reverse". This makes it hard to understand what reversedInstructionsWithoutDebug will do: I've removed the helper to prevent similar mistakes in the future.
- Loading branch information
Showing
4 changed files
with
58 additions
and
12 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
# RUN: llc %s -o - -run-pass=peephole-opt | FileCheck %s | ||
|
||
# Check that we don't optimize out a subs due to areCFlagsAccessedBetweenInstrs | ||
# returning the wrong result; it should check the cneg before the subs which does | ||
# modify cflags. | ||
|
||
# CHECK-LABEL: f | ||
# CHECK: SUBSWrr | ||
# CHECK-NEXT: SUBWrr | ||
# CHECK-NEXT: CSNEGWr | ||
# CHECK-NEXT: SUBSWri | ||
# CHECK-NEXT: CSNEGWr | ||
# CHECK-NEXT: Bcc | ||
|
||
--- | | ||
target datalayout = "e-m:w-p:64:64-i32:32-i64:64-i128:128-n32:64-S128" | ||
target triple = "aarch64-w64-windows-gnu" | ||
|
||
define dso_local void @f() { | ||
ret void | ||
} | ||
|
||
... | ||
--- | ||
name: f | ||
registers: | ||
- { id: 43, class: gpr32, preferred-register: '' } | ||
- { id: 44, class: gpr32, preferred-register: '' } | ||
- { id: 46, class: gpr32, preferred-register: '' } | ||
- { id: 47, class: gpr32, preferred-register: '' } | ||
- { id: 48, class: gpr32common, preferred-register: '' } | ||
- { id: 49, class: gpr32common, preferred-register: '' } | ||
- { id: 50, class: gpr32, preferred-register: '' } | ||
- { id: 51, class: gpr32, preferred-register: '' } | ||
- { id: 52, class: gpr32, preferred-register: '' } | ||
- { id: 53, class: gpr32, preferred-register: '' } | ||
body: | | ||
bb.0: | ||
successors: %bb.0 | ||
%43 = MOVi32imm 1 | ||
%44 = MOVi32imm 1 | ||
%46 = MOVi32imm 1 | ||
%47 = MOVi32imm 1 | ||
%48 = nsw SUBSWrr killed %43, killed %46, implicit-def dead $nzcv | ||
%49 = nsw SUBSWrr killed %44, killed %47, implicit-def dead $nzcv | ||
%50 = SUBSWri %48, 0, 0, implicit-def $nzcv | ||
%51 = CSNEGWr %48, %48, 5, implicit $nzcv | ||
%52 = SUBSWri %49, 0, 0, implicit-def $nzcv | ||
%53 = CSNEGWr %49, %49, 5, implicit $nzcv | ||
Bcc 1, %bb.0, implicit $nzcv | ||
RET_ReallyLR | ||
... |