[X86] Encode global symbol address in small code model

[weiwang]

Summary

In small code model, program and its symbols are linked in the lower 2 GB of the address space. Try encoding global symbol address even when the range is unknown in such case.

[craig.topper]

Is this something we can detect in X86DAGToDAGISel::isSExtAbsoluteSymbolRef? We already have a pattern for X86sub_flag that calls that. Same for a bunch of other instructions.

[wenlei]

Would be good to share benchmark/performance numbers you have for this change too. Thanks.

[weiwang]

Thanks for the comment, Craig.

isSExtAbsoluteSymbolRef does seem to check for the width of immediate. If the immediate can be encoded directly, the node should be replaced with one of the SUB64ri* nodes. I think the X86Wrapper node can be replaced with a corresponding imm node if conditions are met, then the matching can proceed.

[weiwang]

In D89341#2328984, @wenlei wrote:

Would be good to share benchmark/performance numbers you have for this change too. Thanks.

Sure. This change is to address some codegen difference we saw between clang and gcc from internal workloads. Gcc seems to prefer encoding address immediate into cmp in small code model. With the change, we saw 1% perf improvement on average across multiple workloads.

[craig.topper]

In D89341#2328996, @weiwang wrote:

Thanks for the comment, Craig.

isSExtAbsoluteSymbolRef does seem to check for the width of immediate. If the immediate can be encoded directly, the node should be replaced with one of the SUB64ri* nodes. I think the X86Wrapper node can be replaced with a corresponding imm node if conditions are met, then the matching can proceed.

I was thinking that if we don't have range information from getAbsoluteSymbolRange, but the Width passed to isSExtAbsoluteSymbolRef is 32 and the code model is small we could return true?

[weiwang]

In D89341#2331338, @craig.topper wrote:

In D89341#2328996, @weiwang wrote:

Thanks for the comment, Craig.

isSExtAbsoluteSymbolRef does seem to check for the width of immediate. If the immediate can be encoded directly, the node should be replaced with one of the SUB64ri* nodes. I think the X86Wrapper node can be replaced with a corresponding imm node if conditions are met, then the matching can proceed.

I was thinking that if we don't have range information from getAbsoluteSymbolRange, but the Width passed to isSExtAbsoluteSymbolRef is 32 and the code model is small we could return true?

Yes, I think it is possible to do that. The similar check is also done in X86DAGToDAGISel::selectMOV64Imm32.

[weiwang]

Sorry for the long delay. Got held up by some other task.

Update:

1. modified change as suggested.
2. fixed several test cases due to the change.
3. added a new test case.

[craig.topper]

LGTM

llvm/lib/Target/X86/X86InstrCompiler.td

def : Pat<(sub GR16:$src1, i16immSExt8:$src2),
           (SUB16ri8 GR16:$src1, i16immSExt8:$src2)>;
 def : Pat<(sub GR32:$src1, i32immSExt8:$src2),
           (SUB32ri8 GR32:$src1, i32immSExt8:$src2)>;
 def : Pat<(sub GR64:$src1, i64immSExt8:$src2),
           (SUB64ri8 GR64:$src1, i64immSExt8:$src2)>;
 def : Pat<(sub GR64:$src1, i64immSExt32:$src2),
           (SUB64ri32 GR64:$src1, i64immSExt32:$src2)>;
 
+// sub reg, X86Wrapper
+// Adjust the complexity so that it is matched after 'sub reg, imm' patterns.
+let AddedComplexity = -3 in {
+def : Pat<(X86sub_flag GR64:$src1, mov64imm32:$src2),
+          (SUB64ri32 GR64:$src1, mov64imm32:$src2)>;
+}
+
 // 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)>;
 
 // mul reg, reg
 def : Pat<(mul GR16:$src1, GR16:$src2),

llvm/test/CodeGen/X86/critical-edge-split-2.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc < %s -mtriple=x86_64-unknown-unknown | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown --code-model=medium | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown --code-model=small | FileCheck %s --check-prefix=CHECK-SMALL
 
 %0 = type <{ %1, %1 }>
 %1 = type { i8, i8, i8, i8 }
 
 @g_2 = global %0 zeroinitializer
 @g_4 = global %1 zeroinitializer, align 4
 
 ; PR8642
 define i16 @test1(i1 zeroext %C, i8** nocapture %argv) nounwind ssp {
 ; CHECK-LABEL: test1:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    movw $1, %ax
 ; CHECK-NEXT:    testl %edi, %edi
 ; CHECK-NEXT:    jne .LBB0_2
 ; CHECK-NEXT:  # %bb.1: # %cond.false.i
-; CHECK-NEXT:    movl $g_4, %eax
-; CHECK-NEXT:    movl $g_2+4, %ecx
+; CHECK-NEXT:    movabsq $g_4, %rax
+; CHECK-NEXT:    movabsq $g_2, %rcx
+; CHECK-NEXT:    addq $4, %rcx
 ; CHECK-NEXT:    xorl %esi, %esi
 ; CHECK-NEXT:    cmpq %rax, %rcx
 ; CHECK-NEXT:    sete %sil
 ; CHECK-NEXT:    movl $1, %eax
 ; CHECK-NEXT:    xorl %edx, %edx
 ; CHECK-NEXT:    divl %esi
 ; CHECK-NEXT:    movl %edx, %eax
 ; CHECK-NEXT:  .LBB0_2: # %cond.end.i
 ; CHECK-NEXT:    # kill: def $ax killed $ax killed $eax
 ; CHECK-NEXT:    retq
+
+; CHECK-SMALL-LABEL: test1:
+; CHECK-SMALL:       # %bb.0: # %entry
+; CHECK-SMALL-NEXT:    movw $1, %ax
+; CHECK-SMALL-NEXT:    testl %edi, %edi
+; CHECK-SMALL-NEXT:    jne .LBB0_2
+; CHECK-SMALL-NEXT:  # %bb.1: # %cond.false.i
+; CHECK-SMALL-NEXT:    movl $g_2+4, %eax
+; CHECK-SMALL-NEXT:    xorl %ecx, %ecx
+; CHECK-SMALL-NEXT:    cmpq $g_4, %rax
+; CHECK-SMALL-NEXT:    sete %cl
+; CHECK-SMALL-NEXT:    movl $1, %eax
+; CHECK-SMALL-NEXT:    xorl %edx, %edx
+; CHECK-SMALL-NEXT:    divl %ecx
+; CHECK-SMALL-NEXT:    movl %edx, %eax
+; CHECK-SMALL-NEXT:  .LBB0_2: # %cond.end.i
+; CHECK-SMALL-NEXT:    # kill: def $ax killed $ax killed $eax
+; CHECK-SMALL-NEXT:    retq
 entry:
   br i1 %C, label %cond.end.i, label %cond.false.i
 
 cond.false.i:
   br label %cond.end.i
 
 cond.end.i:
   %call1 = phi i16 [ trunc (i32 srem (i32 1, i32 zext (i1 icmp eq (%1* bitcast (i8* getelementptr inbounds (%0, %0* @g_2, i64 0, i32 1, i32 0) to %1*), %1* @g_4) to i32)) to i16), %cond.false.i ], [ 1, %entry ]
   ret i16 %call1
 }