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

[Hans Wennborg via llvm-commits]

hans 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)]>;
----------------
DavidKreitzer wrote:
> 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.)
I don't really like the idea of having a pseudo instruction expand in (too many) different ways, but I also feel I don't have enough experience in the backend to argue my case very well :-)

Already, I would have preferred to just do this with patterns, but I think we need the pseudo instruction for rematerialization to work.

But to have a generic MOV32ri that would expand to different things seems like it's almost working against the code generator somehow. For example, how can it get scheduled effectively?

Also, checking whether EFLAGS is live wouldn't be very efficient. For example, X86InstrInfo::isSafeToClobberEFLAGS only considers a few instructions in each direction for efficiency. I would much prefer to have EFLAGS taken into account when choosing to use the instruction.


I still prefer the current patch. We could later add another pseudo for the push/pop lowering with minsize, and then try to figure out the best way to handle 64-bit.


http://reviews.llvm.org/D14971