[llvm] r265020 - [PowerPC] Remove incorrect use of COPY_TO_REGCLASS in fast isel

Ulrich Weigand via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 31 07:44:51 PDT 2016


Author: uweigand
Date: Thu Mar 31 09:44:50 2016
New Revision: 265020

URL: http://llvm.org/viewvc/llvm-project?rev=265020&view=rev
Log:
[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

Added:
    llvm/trunk/test/CodeGen/PowerPC/fast-isel-fpconv.ll
Modified:
    llvm/trunk/lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp
    llvm/trunk/lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp
    llvm/trunk/lib/Target/PowerPC/PPCFastISel.cpp

Modified: llvm/trunk/lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp?rev=265020&r1=265019&r2=265020&view=diff
==============================================================================
--- llvm/trunk/lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp (original)
+++ llvm/trunk/lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp Thu Mar 31 09:44:50 2016
@@ -136,17 +136,6 @@ void PPCInstPrinter::printInst(const MCI
     return;
   }
   
-  // For fast-isel, a COPY_TO_REGCLASS may survive this long.  This is
-  // used when converting a 32-bit float to a 64-bit float as part of
-  // conversion to an integer (see PPCFastISel.cpp:SelectFPToI()),
-  // as otherwise we have problems with incorrect register classes
-  // in machine instruction verification.  For now, just avoid trying
-  // to print it as such an instruction has no effect (a 32-bit float
-  // in a register is already in 64-bit form, just with lower
-  // precision).  FIXME: Is there a better solution?
-  if (MI->getOpcode() == TargetOpcode::COPY_TO_REGCLASS)
-    return;
-
   if (!printAliasInstr(MI, O))
     printInstruction(MI, O);
   printAnnotation(O, Annot);

Modified: llvm/trunk/lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp?rev=265020&r1=265019&r2=265020&view=diff
==============================================================================
--- llvm/trunk/lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp (original)
+++ llvm/trunk/lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp Thu Mar 31 09:44:50 2016
@@ -105,13 +105,8 @@ public:
   void encodeInstruction(const MCInst &MI, raw_ostream &OS,
                          SmallVectorImpl<MCFixup> &Fixups,
                          const MCSubtargetInfo &STI) const override {
-    // For fast-isel, a float COPY_TO_REGCLASS can survive this long.
-    // It's just a nop to keep the register classes happy, so don't
-    // generate anything.
     unsigned Opcode = MI.getOpcode();
     const MCInstrDesc &Desc = MCII.get(Opcode);
-    if (Opcode == TargetOpcode::COPY_TO_REGCLASS)
-      return;
 
     uint64_t Bits = getBinaryCodeForInstr(MI, Fixups, STI);
 

Modified: llvm/trunk/lib/Target/PowerPC/PPCFastISel.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/PPCFastISel.cpp?rev=265020&r1=265019&r2=265020&view=diff
==============================================================================
--- llvm/trunk/lib/Target/PowerPC/PPCFastISel.cpp (original)
+++ llvm/trunk/lib/Target/PowerPC/PPCFastISel.cpp Thu Mar 31 09:44:50 2016
@@ -1131,14 +1131,13 @@ bool PPCFastISel::SelectFPToI(const Inst
     return false;
 
   // Convert f32 to f64 if necessary.  This is just a meaningless copy
-  // to get the register class right.  COPY_TO_REGCLASS is needed since
-  // a COPY from F4RC to F8RC is converted to a F4RC-F4RC copy downstream.
+  // to get the register class right.
   const TargetRegisterClass *InRC = MRI.getRegClass(SrcReg);
   if (InRC == &PPC::F4RCRegClass) {
     unsigned TmpReg = createResultReg(&PPC::F8RCRegClass);
     BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
-            TII.get(TargetOpcode::COPY_TO_REGCLASS), TmpReg)
-      .addReg(SrcReg).addImm(PPC::F8RCRegClassID);
+            TII.get(TargetOpcode::COPY), TmpReg)
+      .addReg(SrcReg);
     SrcReg = TmpReg;
   }
 

Added: llvm/trunk/test/CodeGen/PowerPC/fast-isel-fpconv.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/PowerPC/fast-isel-fpconv.ll?rev=265020&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/PowerPC/fast-isel-fpconv.ll (added)
+++ llvm/trunk/test/CodeGen/PowerPC/fast-isel-fpconv.ll Thu Mar 31 09:44:50 2016
@@ -0,0 +1,33 @@
+; RUN: llc -mtriple powerpc64-unknown-linux-gnu -fast-isel -O0 < %s | FileCheck %s
+
+; The second fctiwz would use an incorrect input register due to wrong handling
+; of COPY_TO_REGCLASS in the FastISel pass.  Verify that this is fixed.
+
+declare void @func(i32, i32)
+
+define void @test() {
+; CHECK-LABEL: test:
+; CHECK: bl func
+; CHECK-NEXT: nop
+; CHECK: lfs [[REG:[0-9]+]], 
+; CHECK: fctiwz {{[0-9]+}}, [[REG]]
+; CHECK: bl func
+; CHECK-NEXT: nop
+
+  %memPos = alloca float, align 4
+  store float 1.500000e+01, float* %memPos
+  %valPos = load float, float* %memPos
+
+  %memNeg = alloca float, align 4
+  store float -1.500000e+01, float* %memNeg
+  %valNeg = load float, float* %memNeg
+
+  %FloatToIntPos = fptosi float %valPos to i32
+  call void @func(i32 15, i32 %FloatToIntPos)
+
+  %FloatToIntNeg = fptosi float %valNeg to i32
+  call void @func(i32 -15, i32 %FloatToIntNeg)
+
+  ret void
+}
+




More information about the llvm-commits mailing list