Skip to content

Commit bd710cb

Browse files
committed
X86: Remove hack in shouldRewriteCopySrc for subregister handling
In the problematic situation fixed in 61e556d, shouldRewriteCopySrc is called with identical register class arguments, but one has a subregister index. This was very surprising to me, and it probably shouldn't be valid for it to occur. It happens in cases with uncoalescable copies where the register class changes, and further up the chain there is a subregister operand. We could possibly just skip over uncoalsecable instructions in the chain rather than letting this query deal with it (or pre-filter the obvious subreg with same class case). The generic implementation is supposed to account for checking for valid subregisters by checking getMatchingSuperRegClass already, but that was bypassed by the early exit for exact class match. Also adds a reduced mir test demonstrating the exact problematic case.
1 parent 3a2b552 commit bd710cb

File tree

4 files changed

+31
-21
lines changed

4 files changed

+31
-21
lines changed

llvm/lib/CodeGen/TargetRegisterInfo.cpp

+4-1
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,10 @@ static bool shareSameRegisterFile(const TargetRegisterInfo &TRI,
420420
const TargetRegisterClass *SrcRC,
421421
unsigned SrcSubReg) {
422422
// Same register class.
423-
if (DefRC == SrcRC)
423+
//
424+
// When processing uncoalescable copies / bitcasts, it is possible we reach
425+
// here with the same register class, but mismatched subregister indices.
426+
if (DefRC == SrcRC && DefSubReg == SrcSubReg)
424427
return true;
425428

426429
// Both operands are sub registers. Check if they share a register class.

llvm/lib/Target/X86/X86RegisterInfo.cpp

-15
Original file line numberDiff line numberDiff line change
@@ -224,21 +224,6 @@ X86RegisterInfo::getPointerRegClass(const MachineFunction &MF,
224224
}
225225
}
226226

227-
bool X86RegisterInfo::shouldRewriteCopySrc(const TargetRegisterClass *DefRC,
228-
unsigned DefSubReg,
229-
const TargetRegisterClass *SrcRC,
230-
unsigned SrcSubReg) const {
231-
// Prevent rewriting a copy where the destination size is larger than the
232-
// input size. See PR41619.
233-
// FIXME: Should this be factored into the base implementation somehow.
234-
if (DefRC->hasSuperClassEq(&X86::GR64RegClass) && DefSubReg == 0 &&
235-
SrcRC->hasSuperClassEq(&X86::GR64RegClass) && SrcSubReg == X86::sub_32bit)
236-
return false;
237-
238-
return TargetRegisterInfo::shouldRewriteCopySrc(DefRC, DefSubReg,
239-
SrcRC, SrcSubReg);
240-
}
241-
242227
const TargetRegisterClass *
243228
X86RegisterInfo::getGPRsForTailCall(const MachineFunction &MF) const {
244229
const Function &F = MF.getFunction();

llvm/lib/Target/X86/X86RegisterInfo.h

-5
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,6 @@ class X86RegisterInfo final : public X86GenRegisterInfo {
7070
getLargestLegalSuperClass(const TargetRegisterClass *RC,
7171
const MachineFunction &MF) const override;
7272

73-
bool shouldRewriteCopySrc(const TargetRegisterClass *DefRC,
74-
unsigned DefSubReg,
75-
const TargetRegisterClass *SrcRC,
76-
unsigned SrcSubReg) const override;
77-
7873
/// getPointerRegClass - Returns a TargetRegisterClass used for pointer
7974
/// values.
8075
const TargetRegisterClass *
+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
2+
# RUN: llc -mtriple=x86_64-- -mattr=+avx2 -run-pass=peephole-opt -o - %s | FileCheck %s
3+
4+
# When trying to coalesce the operand of VMOVSDto64rr, a query would
5+
# be made with the same register class but the source has a
6+
# subregister and the result does not.
7+
---
8+
name: uncoalescable_copy_queries_same_regclass_with_only_one_subreg
9+
tracksRegLiveness: true
10+
isSSA: true
11+
body: |
12+
bb.0:
13+
liveins: $rax
14+
15+
; CHECK-LABEL: name: uncoalescable_copy_queries_same_regclass_with_only_one_subreg
16+
; CHECK: liveins: $rax
17+
; CHECK-NEXT: {{ $}}
18+
; CHECK-NEXT: [[COPY:%[0-9]+]]:gr64 = COPY $rax
19+
; CHECK-NEXT: [[COPY1:%[0-9]+]]:vr128 = COPY [[COPY]].sub_32bit
20+
; CHECK-NEXT: [[VMOVSDto64rr:%[0-9]+]]:gr64 = VMOVSDto64rr [[COPY1]]
21+
; CHECK-NEXT: RET 0, implicit [[VMOVSDto64rr]]
22+
%0:gr64 = COPY $rax
23+
%1:vr128 = COPY %0.sub_32bit
24+
%2:gr64 = VMOVSDto64rr %1
25+
RET 0, implicit %2
26+
27+
...

0 commit comments

Comments
 (0)