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

[Mingming Liu via Phabricator via llvm-commits]

mingmingl accepted this revision.
mingmingl added a comment.
This revision is now accepted and ready to land.

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 <https://reviews.llvm.org/D124118> exposes the SF usage issue, so this patch is reasonable.

Besides, D128122 <https://reviews.llvm.org/D128122> 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



================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:4463-4464
         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
----------------
(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


================
Comment at: llvm/test/CodeGen/X86/pr56103.ll:3
+
+; Check the test instruction won't be optimizated by peephole opt.
+; CHECK: testq
----------------
nit pick:

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128122/new/

https://reviews.llvm.org/D128122