[X86] Don't avoid folding multiple use sign extended 8-bit immediate into instructions under optsize.

[craig.topper]

Summary

Under optsize we try to avoid folding immediates into instructions under optsize. But if the immediate is 16-bits or 32 bits, but can be encoded as an 8-bit immediate we don't save enough from disabling the folding unless the immediate has enough uses to make up for the size of the move which is either 3 bytes or 5 bytes since there are no sign extended 8-bit moves. We would also save something if the immediate was a live out of the basic block and thus a move was unavoidable, but that would require a more advanced heuristic than just counting uses.

Note we only avoid folding multiple use immediates into the patterns that use X86ISD::ADD/SUB/XOR/OR/AND/CMP/ADC/SBB nodes and not the more common ISD::ADD/SUB/XOR/OR/AND nodes.

[craig.topper]

test/CodeGen/X86/immediate_merging64.ll

14 ↗

(On Diff #191211)

This is weird. Once the compare selected. The use count on the -1 constant should have dropped and allowed it fold into the store. Unless isel selected the store before the cmp. I think this demonstrates a larger flaw in the use count check as part of instruction selection. The use count of constants can be influenced by instruction selection order.

[spatel]

Seems good, but let Simon have a look too in case there's some uarch concern that I'm not aware of.

test/CodeGen/X86/immediate_merging.ll

14–15 ↗

(On Diff #191211)

Partly stale comment. Add a line for 8-bit exception.

[RKSimon]

LGTM

test/CodeGen/X86/immediate_merging64.ll

14 ↗

(On Diff #191211)

Please can you raise a bug on this - we're collecting a lot of immediate constant bugs at the moment.......

llvm/trunk/lib/Target/X86/X86InstrArithmetic.td

 
 def invalid_node : SDNode<"<<invalid_node>>", SDTIntLeaf,[],"<<invalid_node>>">;
 
 
 def Xi8  : X86TypeInfo<i8, "b", GR8, loadi8, i8mem,
                        Imm8, i8imm, relocImm8_su, i8imm, invalid_node,
                        0, OpSizeFixed, 0>;
 def Xi16 : X86TypeInfo<i16, "w", GR16, loadi16, i16mem,
-                       Imm16, i16imm, relocImm16_su, i16i8imm, i16immSExt8_su,
+                       Imm16, i16imm, relocImm16_su, i16i8imm, i16immSExt8,
                        1, OpSize16, 0>;
 def Xi32 : X86TypeInfo<i32, "l", GR32, loadi32, i32mem,
-                       Imm32, i32imm, relocImm32_su, i32i8imm, i32immSExt8_su,
+                       Imm32, i32imm, relocImm32_su, i32i8imm, i32immSExt8,
                        1, OpSize32, 0>;
 def Xi64 : X86TypeInfo<i64, "q", GR64, loadi64, i64mem,
-                       Imm32S, i64i32imm, i64relocImmSExt32_su, i64i8imm, i64immSExt8_su,
+                       Imm32S, i64i32imm, i64relocImmSExt32_su, i64i8imm, i64immSExt8,
                        1, OpSizeFixed, 1>;
 
 /// ITy - This instruction base class takes the type info for the instruction.
 /// Using this, it:
 /// 1. Concatenates together the instruction mnemonic with the appropriate
 ///    suffix letter, a tab, and the arguments.
 /// 2. Infers whether the instruction should have a 0x66 prefix byte.
 /// 3. Infers whether the instruction should have a 0x40 REX_W prefix.

llvm/trunk/lib/Target/X86/X86InstrCompiler.td

 
 // sub 0, reg
 def : Pat<(X86sub_flag 0, GR8 :$src), (NEG8r  GR8 :$src)>;
 def : Pat<(X86sub_flag 0, GR16:$src), (NEG16r GR16:$src)>;
 def : Pat<(X86sub_flag 0, GR32:$src), (NEG32r GR32:$src)>;
 def : Pat<(X86sub_flag 0, GR64:$src), (NEG64r GR64:$src)>;
 
 // sub reg, relocImm
-def : Pat<(X86sub_flag GR64:$src1, i64relocImmSExt8_su:$src2),
-          (SUB64ri8 GR64:$src1, i64relocImmSExt8_su:$src2)>;
+def : Pat<(X86sub_flag GR64:$src1, i64relocImmSExt8:$src2),
+          (SUB64ri8 GR64:$src1, i64relocImmSExt8:$src2)>;
 
 // mul reg, reg
 def : Pat<(mul GR16:$src1, GR16:$src2),
           (IMUL16rr GR16:$src1, GR16:$src2)>;
 def : Pat<(mul GR32:$src1, GR32:$src2),
           (IMUL32rr GR32:$src1, GR32:$src2)>;
 def : Pat<(mul GR64:$src1, GR64:$src2),
           (IMUL64rr GR64:$src1, GR64:$src2)>;

llvm/trunk/lib/Target/X86/X86InstrInfo.td

 }]>;
 def relocImm16_su : PatLeaf<(i16 relocImm), [{
     return !shouldAvoidImmediateInstFormsForSize(N);
 }]>;
 def relocImm32_su : PatLeaf<(i32 relocImm), [{
     return !shouldAvoidImmediateInstFormsForSize(N);
 }]>;
 
-def i16immSExt8_su : PatLeaf<(i16immSExt8), [{
-    return !shouldAvoidImmediateInstFormsForSize(N);
-}]>;
-def i32immSExt8_su : PatLeaf<(i32immSExt8), [{
-    return !shouldAvoidImmediateInstFormsForSize(N);
-}]>;
-def i64immSExt8_su : PatLeaf<(i64immSExt8), [{
-    return !shouldAvoidImmediateInstFormsForSize(N);
-}]>;
-
-def i64relocImmSExt8_su : PatLeaf<(i64relocImmSExt8), [{
-    return !shouldAvoidImmediateInstFormsForSize(N);
-}]>;
 def i64relocImmSExt32_su : PatLeaf<(i64relocImmSExt32), [{
     return !shouldAvoidImmediateInstFormsForSize(N);
 }]>;
 
 // i64immZExt32 predicate - True if the 64-bit immediate fits in a 32-bit
 // unsigned field.
 def i64immZExt32 : ImmLeaf<i64, [{ return isUInt<32>(Imm); }]>;
 

llvm/trunk/test/CodeGen/X86/immediate_merging.ll

 ; Test -Os to make sure immediates with multiple users don't get pulled in to
 ; instructions.
 define i32 @foo() optsize {
 ; X86-LABEL: foo:
 ; X86:       # %bb.0: # %entry
 ; X86-NEXT:    movl $1234, %eax # imm = 0x4D2
 ; X86-NEXT:    movl %eax, a
 ; X86-NEXT:    movl %eax, b
-; X86-NEXT:    movl $12, %eax
-; X86-NEXT:    movl %eax, c
-; X86-NEXT:    cmpl %eax, e
+; X86-NEXT:    movl $12, c
+; X86-NEXT:    cmpl $12, e
 ; X86-NEXT:    jne .LBB0_2
 ; X86-NEXT:  # %bb.1: # %if.then
 ; X86-NEXT:    movl $1, x
 ; X86-NEXT:  .LBB0_2: # %if.end
 ; X86-NEXT:    movl $1234, f # imm = 0x4D2
 ; X86-NEXT:    movl $555, %eax # imm = 0x22B
 ; X86-NEXT:    movl %eax, h
 ; X86-NEXT:    addl %eax, i
 ; X86-NEXT:    xorl %eax, %eax
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: foo:
 ; X64:       # %bb.0: # %entry
 ; X64-NEXT:    movl $1234, %eax # imm = 0x4D2
 ; X64-NEXT:    movl %eax, {{.*}}(%rip)
 ; X64-NEXT:    movl %eax, {{.*}}(%rip)
-; X64-NEXT:    movl $12, %eax
-; X64-NEXT:    movl %eax, {{.*}}(%rip)
-; X64-NEXT:    cmpl %eax, {{.*}}(%rip)
+; X64-NEXT:    movl $12, {{.*}}(%rip)
+; X64-NEXT:    cmpl $12, {{.*}}(%rip)
 ; X64-NEXT:    jne .LBB0_2
 ; X64-NEXT:  # %bb.1: # %if.then
 ; X64-NEXT:    movl $1, {{.*}}(%rip)
 ; X64-NEXT:  .LBB0_2: # %if.end
 ; X64-NEXT:    movl $1234, {{.*}}(%rip) # imm = 0x4D2
 ; X64-NEXT:    movl $555, %eax # imm = 0x22B
 ; X64-NEXT:    movl %eax, {{.*}}(%rip)
 ; X64-NEXT:    addl %eax, {{.*}}(%rip)

llvm/trunk/test/CodeGen/X86/immediate_merging64.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
 
 ; Check that multiple instances of 64-bit constants encodable as
 ; 32-bit immediates are merged for code size savings.
 
 ; Immediates with multiple users should not be pulled into instructions when
 ; optimizing for code size.
 define i1 @imm_multiple_users(i64 %a, i64* %b) optsize {
 ; CHECK-LABEL: imm_multiple_users:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    movq $-1, %rax
 ; CHECK-NEXT:    movq %rax, (%rsi)
-; CHECK-NEXT:    cmpq %rax, %rdi
+; CHECK-NEXT:    cmpq $-1, %rdi
 ; CHECK-NEXT:    sete %al
 ; CHECK-NEXT:    retq
   store i64 -1, i64* %b, align 8
   %cmp = icmp eq i64 %a, -1
   ret i1 %cmp
 }
 
 declare void @llvm.memset.p0i8.i64(i8* nocapture, i8, i64, i1)