[PATCH] Implement mips fast-isel select statement
Daniel Sanders
daniel.sanders at imgtec.com
Tue May 12 04:06:38 PDT 2015
================
Comment at: lib/Target/Mips/MipsFastISel.cpp:928-934
@@ +927,9 @@
+ return false;
+ case MVT::i1:
+ case MVT::i8:
+ case MVT::i16:
+ case MVT::i32:
+ CondMovOpc = Mips::MOVN_I_I;
+ RC = &Mips::GPR32RegClass;
+ break;
+ case MVT::f32:
----------------
We don't need to check each integer type explictly. The generated code is correct for all integers of GPR size or smaller.
================
Comment at: lib/Target/Mips/MipsFastISel.cpp:947
@@ +946,3 @@
+ const Value *Cond = SI->getCondition();
+ (void)Cond;
+ unsigned Src1Reg = getRegForValue(SI->getTrueValue());
----------------
Nit: Useless statement
================
Comment at: lib/Target/Mips/MipsFastISel.cpp:960
@@ +959,3 @@
+ return false;
+ emitInst(TargetOpcode::COPY, TempReg).addReg(Src2Reg);
+ MachineInstrBuilder MI = emitInst(CondMovOpc, ResultReg)
----------------
I think this is only needed if Src2Reg != DestReg. Is it possible for this code to emit something like:
move $2, $2
movn $2, $3, $4
================
Comment at: lib/Target/Mips/MipsFastISel.cpp:964
@@ +963,3 @@
+ .addReg(CondReg)
+ .addReg(TempReg, RegState::Implicit);
+ MI->tieOperands(0, 3);
----------------
TempReg isn't implicitly read. It's explicit in our instruction definition:
(outs DRC:$rd), (ins DRC:$rs, CRC:$rt, DRC:$F)
================
Comment at: lib/Target/Mips/MipsFastISel.cpp:1660-1679
@@ -1600,2 +1659,22 @@
+unsigned MipsFastISel::fastEmitInst_rr(unsigned MachineInstOpcode,
+ const TargetRegisterClass *RC,
+ unsigned Op0, bool Op0IsKill,
+ unsigned Op1, bool Op1IsKill) {
+ if (MachineInstOpcode == Mips::MUL) {
+ unsigned ResultReg = createResultReg(RC);
+ const MCInstrDesc &II = TII.get(MachineInstOpcode);
+ Op0 = constrainOperandRegClass(II, Op0, II.getNumDefs());
+ Op1 = constrainOperandRegClass(II, Op1, II.getNumDefs() + 1);
+ BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, II, ResultReg)
+ .addReg(Op0, getKillRegState(Op0IsKill))
+ .addReg(Op1, getKillRegState(Op1IsKill))
+ .addReg(Mips::HI0, RegState::ImplicitDefine | RegState::Dead)
+ .addReg(Mips::LO0, RegState::ImplicitDefine | RegState::Dead);
+ return ResultReg;
+ }
+ return FastISel::fastEmitInst_rr(MachineInstOpcode, RC, Op0, Op0IsKill, Op1,
+ Op1IsKill);
+}
+
namespace llvm {
----------------
Why is this chunk in this patch? It looks we have two patches in one review.
================
Comment at: test/CodeGen/Mips/Fast-ISel/sel1.ll:22
@@ +21,3 @@
+; Function Attrs: noinline nounwind
+define void @foo() #0 {
+entry:
----------------
At this point we should have support for arguments and returns. Could you simplify the test using those?
I don't mind if we clean this up later but I'd like to have as little noise as possible in the tests.
http://reviews.llvm.org/D6774
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list