Skip to content
This repository was archived by the owner on Mar 28, 2020. It is now read-only.

Commit ca50bf5

Browse files
committed
[PowerPC] Remove incorrect use of COPY_TO_REGCLASS in fast isel
The fast isel pass currently emits a COPY_TO_REGCLASS node to convert from a F4RC to a F8RC register class during conversion of a floating-point number to integer. There is actually no support in the common code instruction printers to emit COPY_TO_REGCLASS nodes, so the PowerPC back-end has special code there to simply ignore COPY_TO_REGCLASS. This is correct *if and only if* the source and destination registers of COPY_TO_REGCLASS are the same (except for the different register class). But nothing guarantees this to be the case, and if the register allocator does end up allocating source and destination to different registers after all, the back-end simply generates incorrect code. I've included a test case that shows such incorrect code generation. However, it seems that COPY_TO_REGCLASS is actually not intended to be used at the MI layer at all. It is used during SelectionDAG, but always lowered to a plain COPY before emitting MI. Other back-end's fast isel passes never emit COPY_TO_REGCLASS at all. I suspect it is simply wrong for the PowerPC back-end to emit it here. This patch changes the PowerPC back-end to directly emit COPY instead of COPY_TO_REGCLASS and removes the special handling in the instruction printers. Differential Revision: http://reviews.llvm.org/D18605 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@265020 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 2fcdb70 commit ca50bf5

File tree

4 files changed

+36
-20
lines changed

4 files changed

+36
-20
lines changed

lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -136,17 +136,6 @@ void PPCInstPrinter::printInst(const MCInst *MI, raw_ostream &O,
136136
return;
137137
}
138138

139-
// For fast-isel, a COPY_TO_REGCLASS may survive this long. This is
140-
// used when converting a 32-bit float to a 64-bit float as part of
141-
// conversion to an integer (see PPCFastISel.cpp:SelectFPToI()),
142-
// as otherwise we have problems with incorrect register classes
143-
// in machine instruction verification. For now, just avoid trying
144-
// to print it as such an instruction has no effect (a 32-bit float
145-
// in a register is already in 64-bit form, just with lower
146-
// precision). FIXME: Is there a better solution?
147-
if (MI->getOpcode() == TargetOpcode::COPY_TO_REGCLASS)
148-
return;
149-
150139
if (!printAliasInstr(MI, O))
151140
printInstruction(MI, O);
152141
printAnnotation(O, Annot);

lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -105,13 +105,8 @@ class PPCMCCodeEmitter : public MCCodeEmitter {
105105
void encodeInstruction(const MCInst &MI, raw_ostream &OS,
106106
SmallVectorImpl<MCFixup> &Fixups,
107107
const MCSubtargetInfo &STI) const override {
108-
// For fast-isel, a float COPY_TO_REGCLASS can survive this long.
109-
// It's just a nop to keep the register classes happy, so don't
110-
// generate anything.
111108
unsigned Opcode = MI.getOpcode();
112109
const MCInstrDesc &Desc = MCII.get(Opcode);
113-
if (Opcode == TargetOpcode::COPY_TO_REGCLASS)
114-
return;
115110

116111
uint64_t Bits = getBinaryCodeForInstr(MI, Fixups, STI);
117112

lib/Target/PowerPC/PPCFastISel.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,14 +1131,13 @@ bool PPCFastISel::SelectFPToI(const Instruction *I, bool IsSigned) {
11311131
return false;
11321132

11331133
// Convert f32 to f64 if necessary. This is just a meaningless copy
1134-
// to get the register class right. COPY_TO_REGCLASS is needed since
1135-
// a COPY from F4RC to F8RC is converted to a F4RC-F4RC copy downstream.
1134+
// to get the register class right.
11361135
const TargetRegisterClass *InRC = MRI.getRegClass(SrcReg);
11371136
if (InRC == &PPC::F4RCRegClass) {
11381137
unsigned TmpReg = createResultReg(&PPC::F8RCRegClass);
11391138
BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
1140-
TII.get(TargetOpcode::COPY_TO_REGCLASS), TmpReg)
1141-
.addReg(SrcReg).addImm(PPC::F8RCRegClassID);
1139+
TII.get(TargetOpcode::COPY), TmpReg)
1140+
.addReg(SrcReg);
11421141
SrcReg = TmpReg;
11431142
}
11441143

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
; RUN: llc -mtriple powerpc64-unknown-linux-gnu -fast-isel -O0 < %s | FileCheck %s
2+
3+
; The second fctiwz would use an incorrect input register due to wrong handling
4+
; of COPY_TO_REGCLASS in the FastISel pass. Verify that this is fixed.
5+
6+
declare void @func(i32, i32)
7+
8+
define void @test() {
9+
; CHECK-LABEL: test:
10+
; CHECK: bl func
11+
; CHECK-NEXT: nop
12+
; CHECK: lfs [[REG:[0-9]+]],
13+
; CHECK: fctiwz {{[0-9]+}}, [[REG]]
14+
; CHECK: bl func
15+
; CHECK-NEXT: nop
16+
17+
%memPos = alloca float, align 4
18+
store float 1.500000e+01, float* %memPos
19+
%valPos = load float, float* %memPos
20+
21+
%memNeg = alloca float, align 4
22+
store float -1.500000e+01, float* %memNeg
23+
%valNeg = load float, float* %memNeg
24+
25+
%FloatToIntPos = fptosi float %valPos to i32
26+
call void @func(i32 15, i32 %FloatToIntPos)
27+
28+
%FloatToIntNeg = fptosi float %valNeg to i32
29+
call void @func(i32 -15, i32 %FloatToIntNeg)
30+
31+
ret void
32+
}
33+

0 commit comments

Comments
 (0)