[PATCH] D35737: [GSel]: Support Widening G_ICMP's destination operand.

[Volkan Keles via Phabricator via llvm-commits]

volkan added a comment.

Could you add a test case or an explanation if it's not possible to add a test case?



================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:611
+      MIRBuilder.buildInstr(TargetOpcode::G_FPEXT, Op0Ext, MI.getOperand(2).getReg());
+      MIRBuilder.buildInstr(TargetOpcode::G_FPEXT, Op1Ext, MI.getOperand(3).getReg());
+    }
----------------
Could you use local variables for the operands to reduce duplication?


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:617
+    if (TypeIdx == 0)
+      MIRBuilder.buildInstr(TargetOpcode::G_FPTRUNC, MI.getOperand(0).getReg(),
+                            DstReg);
----------------
The result type is boolean. Why do we need to build a G_FPTRUNC  instead of G_TRUNC? 


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:628
+      Op0Ext = MI.getOperand(2).getReg();
+      Op1Ext = MI.getOperand(3).getReg();
+      DstReg = MRI.createGenericVirtualRegister(WideTy);
----------------
Same here.


https://reviews.llvm.org/D35737