[PATCH] D14971: X86: Emit smaller code for moving 8-bit immediates

[David Kreitzer via llvm-commits]

DavidKreitzer added inline comments.

================
Comment at: lib/Target/X86/X86InstrCompiler.td:270
@@ +269,3 @@
+  let Defs = [EFLAGS], isReMaterializable = 1, isPseudo = 1 in {
+    def MOV32r1 : I<0, Pseudo, (outs GR32:$dst), (ins), "",
+                        [(set GR32:$dst, 1)]>;
----------------
I certainly can't argue with the logic that what you are adding is consistent with MOV32r0. And from that perspective, I think your patch is fine the way it is. I just wonder if using MOV32ri universally and having a late expansion that chooses between the many possible implementations is a better long term direction.

I understand your point about the EFLAGS clobber, but note that the pessimism works both ways.  The MOV32r0 prevents transformations that would make EFLAGS live across it, e.g. to remove a redundant compare. We'd probably prefer to remove the redundant compare & use "mov $0, %eax" to generate the 0 than leave the redundant compare & use "xor %eax, %eax". (Of course, you could specifically check for MOV32r0 and convert it to MOV32ri when the transformation happens just like X86InstrInfo::rematerialize is doing, but you could do the same sort of thing with the MOV32ri representation.)

================
Comment at: lib/Target/X86/X86InstrInfo.cpp:2475
@@ -2474,5 +2474,3 @@
                                  const TargetRegisterInfo &TRI) const {
-  // MOV32r0 is implemented with a xor which clobbers condition code.
-  // Re-materialize it as movri instructions to avoid side effects.
-  unsigned Opc = Orig->getOpcode();
-  if (Opc == X86::MOV32r0 && !isSafeToClobberEFLAGS(MBB, I)) {
+  bool ClobbersEFLAGS = false;
+  for (const MachineOperand &MO : Orig->operands()) {
----------------
I like this improvement, thanks!


http://reviews.llvm.org/D14971