[PATCH] D37018: [GISel]: Legalize G_PHIs

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added a comment.

Hi Aditya,

Looks mostly good.
Couple of comments inlined + we need more tests and reduce them (e.g., remove the frameInfo stuff and so on).

Cheers,
-Quentin



================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:668
+      auto NonTermIt = std::prev(MBB.getFirstTerminator());
+      MIRBuilder.setInstr(*NonTermIt, true);
+      MachineInstr *DefMI = MRI.getVRegDef(Reg);
----------------
NonTermIt may not be an instruction, thus we cannot dereference it.
- Use setInsertPt instead.
- Add a test case with an empty predecessor block


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:676
+        MIB = MIRBuilder.buildAnyExt(WideTy, Reg);
+      }
+      return MIB->getOperand(0).getReg();
----------------
No bracket for one line block


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:690
+      MIB.addReg(P.first).addMBB(P.second);
+    }
+    MIRBuilder.buildTrunc(MI.getOperand(0).getReg(),
----------------
Can't we merge the two loops and get rid of the temporary NewRegs.


Repository:
  rL LLVM

https://reviews.llvm.org/D37018