[PATCH] D15549: [X86] Use push-pop for materializing small constants under 'minsize'

Hans Wennborg via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 16 16:25:04 PST 2015


hans added inline comments.

================
Comment at: test/CodeGen/X86/materialize.ll:43
@@ +42,3 @@
+
+; On 64-bit we don't do xor-inc yet, so push-pop it is.
+; CHECK64-LABEL: one32_minsize:
----------------
DavidKreitzer wrote:
> The reason for deferring the use of xor-inc/dec on 64-bit was the possible REX prefixes, right? You'll have the same problem with the pop, though it will only cost one byte rather than two. Is that one-byte difference the reason you allow push/pop currently but avoid xor-inc/dec?
Exactly, even with REX, push/pop is always a win.



================
Comment at: test/CodeGen/X86/materialize.ll:45
@@ +44,3 @@
+; CHECK64-LABEL: one32_minsize:
+; CHECK64:       pushl $1
+; CHECK64:       .cfi_adjust_cfa_offset 4
----------------
DavidKreitzer wrote:
> Hmmm, pushl and popl are not valid instructions in 64-bit mode. They should instead be pushq and popq, and the CFA adjustments should be +/-8. It looks like this is just a problem in the test, as the code to generate them looks fine.
> 
> 
Hmm, it's not just the test. It's selecting the MOV32ImmSExti8 instruction. I'll fix that.

And I now realize that the instruction selector doesn't realize it can use MOV64ImmSExti8 to get a 32-bit constant. I tried to add a pattern for that, but failed so far.. :-/


http://reviews.llvm.org/D15549





More information about the llvm-commits mailing list