[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

6

Drop dso_local and unnamed-addr

49

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–4464

(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

3

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

+; RUN: llc < %s -mtriple=x86_64-linux-generic | FileCheck %s
+
+; Check the test instruction won't be optimizated by peephole opt.
+; CHECK: testq
+
+@e = dso_local local_unnamed_addr global i16 0, align 2
+@a = dso_local local_unnamed_addr global i32 0, align 4
+@c = dso_local local_unnamed_addr global i32 0, align 4
+@b = dso_local local_unnamed_addr global i64 0, align 8
+
+; Function Attrs: nounwind uwtable
+define dso_local i32 @main() local_unnamed_addr #0 {
+entry:
+  store i16 1, ptr @e, align 2, !tbaa !5
+  store i64 1, ptr @b, align 8, !tbaa !9
+  %0 = load i32, ptr @a, align 4, !tbaa !11
+  %shr = ashr i32 -2, %0
+  %1 = load i32, ptr @c, align 4, !tbaa !11
+  %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
+}
+
+; Function Attrs: noreturn nounwind
+declare void @abort() local_unnamed_addr #1
+
+attributes #0 = { nounwind uwtable "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+attributes #1 = { noreturn nounwind "frame-pointer"="none" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+attributes #2 = { noreturn nounwind }
+
+!llvm.module.flags = !{!0, !1, !2, !3}
+!llvm.ident = !{!4}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{i32 7, !"PIC Level", i32 2}
+!2 = !{i32 7, !"PIE Level", i32 2}
+!3 = !{i32 7, !"uwtable", i32 2}
+!4 = !{!"clang version 15.0.0 (https://github.com/llvm/llvm-project.git e4361089ff5fc4bf1400f650876edd2b267a49af)"}
+!5 = !{!6, !6, i64 0}
+!6 = !{!"short", !7, i64 0}
+!7 = !{!"omnipotent char", !8, i64 0}
+!8 = !{!"Simple C/C++ TBAA"}
+!9 = !{!10, !10, i64 0}
+!10 = !{!"long", !7, i64 0}
+!11 = !{!12, !12, i64 0}
+!12 = !{!"int", !7, i64 0}