[X86] Make sure SF is updated when optimizing for `jg/jge/jl/jle`

[pengfei]

Summary

This fixes issue #56103.

[craig.topper]

llvm/test/CodeGen/X86/pr56103.ll

7

Drop dso_local and unnamed-addr

50

Can we clean up the attributes and metadata nodes?

[mingmingl]

thanks for root cause and quick fix! lgtm after resolving others' comments.

As mentioned in issue 56103[1], a diff of codegen in 14.0.0 and truck proves D124118 exposes the SF usage issue, so this patch is reasonable.

Besides, D124118 could update ClearsOverflowFlag; and the OF bit usage is already properly handled [2] so no other EFLAGS usage to worry about (otherwise I'd be glad to send a similar patch to this).

[1] https://github.com/llvm/llvm-project/issues/56103#issuecomment-1159575344
[2] according to the table http://unixwiz.net/techtips/x86-jumps.html

llvm/lib/Target/X86/X86InstrInfo.cpp

4463–4469

(Just to add some reference, no action items)

According to the table [1], JL JLE, JG and JGE uses SF bit, besides JS and JNS.

JS and JNS are handled around line 4478, so this should handle all jump instructions.

[1] http://unixwiz.net/techtips/x86-jumps.html and https://en.wikibooks.org/wiki/X86_Assembly/Control_Flow#Unconditional_Jumps

llvm/test/CodeGen/X86/pr56103.ll

4

nit pick:

Do we want to use utils/update_llc_test_checks.py to generate (more) assertions automatically?

[pengfei]

Address review comments, thanks @craig.topper and @mingmingl!

llvm/lib/Target/X86/X86InstrInfo.cpp

for (MachineInstr &Instr : make_range(AfterCmpInstr, CmpMBB.end())) {
     X86::CondCode ReplacementCC = X86::COND_INVALID;
     if (MI) {
       switch (OldCC) {
       default: break;
       case X86::COND_A: case X86::COND_AE:
       case X86::COND_B: case X86::COND_BE:
         // CF is used, we can't perform this optimization.
         return false;
       case X86::COND_G: case X86::COND_GE:
       case X86::COND_L: case X86::COND_LE:
+        // If SF is used, but the instruction doesn't update the SF, then we
+        // can't do the optimization.
+        if (NoSignFlag)
+          return false;
+        LLVM_FALLTHROUGH;
       case X86::COND_O: case X86::COND_NO:
         // If OF is used, the instruction needs to clear it like CmpZero does.
         if (!ClearsOverflowFlag)
           return false;
         break;
       case X86::COND_S: case X86::COND_NS:
         // If SF is used, but the instruction doesn't update the SF, then we
         // can't do the optimization.

llvm/test/CodeGen/X86/pr56103.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-linux-generic | FileCheck %s
+
+@e = global i16 0, align 2
+@a = global i32 0, align 4
+@c = global i32 0, align 4
+@b = global i64 0, align 8
+
+; Check the test instruction won't be optimizated by peephole opt.
+
+define dso_local i32 @main() nounwind {
+; CHECK-LABEL: main:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    movq e@GOTPCREL(%rip), %rax
+; CHECK-NEXT:    movw $1, (%rax)
+; CHECK-NEXT:    movq b@GOTPCREL(%rip), %rax
+; CHECK-NEXT:    movq $1, (%rax)
+; CHECK-NEXT:    movq a@GOTPCREL(%rip), %rax
+; CHECK-NEXT:    movl (%rax), %ecx
+; CHECK-NEXT:    movl $-2, %eax
+; CHECK-NEXT:    sarl %cl, %eax
+; CHECK-NEXT:    movq c@GOTPCREL(%rip), %rdx
+; CHECK-NEXT:    movl (%rdx), %edx
+; CHECK-NEXT:    decl %edx
+; CHECK-NEXT:    movzwl %ax, %eax
+; CHECK-NEXT:    decl %eax
+; CHECK-NEXT:    xorl %edx, %eax
+; CHECK-NEXT:    notl %ecx
+; CHECK-NEXT:    andl %eax, %ecx
+; CHECK-NEXT:    testq %rcx, %rcx
+; CHECK-NEXT:    jle .LBB0_2
+; CHECK-NEXT:  # %bb.1: # %if.end
+; CHECK-NEXT:    xorl %eax, %eax
+; CHECK-NEXT:    popq %rcx
+; CHECK-NEXT:    retq
+; CHECK-NEXT:  .LBB0_2: # %if.then
+; CHECK-NEXT:    callq abort@PLT
+entry:
+  store i16 1, ptr @e, align 2
+  store i64 1, ptr @b, align 8
+  %0 = load i32, ptr @a, align 4
+  %shr = ashr i32 -2, %0
+  %1 = load i32, ptr @c, align 4
+  %sub = add i32 %1, -1
+  %conv2 = zext i32 %sub to i64
+  %2 = and i32 %shr, 65535
+  %conv3 = zext i32 %2 to i64
+  %sub4 = add nsw i64 %conv3, -1
+  %xor = xor i64 %sub4, %conv2
+  %neg5 = xor i32 %0, -1
+  %conv6 = sext i32 %neg5 to i64
+  %and = and i64 %xor, %conv6
+  %cmp = icmp slt i64 %and, 1
+  br i1 %cmp, label %if.then, label %if.end
+
+if.then:                                          ; preds = %entry
+  call void @abort() #2
+  unreachable
+
+if.end:                                           ; preds = %entry
+  ret i32 0
+}
+
+declare void @abort()