[X86][APX] Handle AND_NF instruction for compare peephole

[e-kud]

No description provided.

[llvmbot]

@llvm/pr-subscribers-backend-x86

Author: Evgenii Kudriashov (e-kud)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/136233.diff

2 Files Affected:

• (modified) llvm/lib/Target/X86/X86InstrInfo.cpp (+8-8)
• (modified) llvm/test/CodeGen/X86/apx/nf-regressions.ll (+33)

diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp
index f595642d734e8..32a94e6ab0594 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.cpp
+++ b/llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -1035,12 +1035,11 @@ inline static bool isTruncatedShiftCountForLEA(unsigned ShAmt) {
   return ShAmt < 4 && ShAmt > 0;
 }
 
-static bool findRedundantFlagInstr(MachineInstr &CmpInstr,
-                                   MachineInstr &CmpValDefInstr,
-                                   const MachineRegisterInfo *MRI,
-                                   MachineInstr **AndInstr,
-                                   const TargetRegisterInfo *TRI,
-                                   bool &NoSignFlag, bool &ClearsOverflowFlag) {
+static bool
+findRedundantFlagInstr(MachineInstr &CmpInstr, MachineInstr &CmpValDefInstr,
+                       const MachineRegisterInfo *MRI, MachineInstr **AndInstr,
+                       const TargetRegisterInfo *TRI, const X86Subtarget &ST,
+                       bool &NoSignFlag, bool &ClearsOverflowFlag) {
   if (!(CmpValDefInstr.getOpcode() == X86::SUBREG_TO_REG &&
         CmpInstr.getOpcode() == X86::TEST64rr) &&
       !(CmpValDefInstr.getOpcode() == X86::COPY &&
@@ -1103,7 +1102,8 @@ static bool findRedundantFlagInstr(MachineInstr &CmpInstr,
   if (VregDefInstr->getParent() != CmpValDefInstr.getParent())
     return false;
 
-  if (X86::isAND(VregDefInstr->getOpcode())) {
+  if (X86::isAND(VregDefInstr->getOpcode()) &&
+      (!ST.hasNF() || VregDefInstr->modifiesRegister(X86::EFLAGS, TRI))) {
     // Get a sequence of instructions like
     //   %reg = and* ...                    // Set EFLAGS
     //   ...                                // EFLAGS not changed
@@ -5433,7 +5433,7 @@ bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
         MachineInstr *AndInstr = nullptr;
         if (IsCmpZero &&
             findRedundantFlagInstr(CmpInstr, Inst, MRI, &AndInstr, TRI,
-                                   NoSignFlag, ClearsOverflowFlag)) {
+                                   Subtarget, NoSignFlag, ClearsOverflowFlag)) {
           assert(AndInstr != nullptr && X86::isAND(AndInstr->getOpcode()));
           MI = AndInstr;
           break;
diff --git a/llvm/test/CodeGen/X86/apx/nf-regressions.ll b/llvm/test/CodeGen/X86/apx/nf-regressions.ll
index 40503d85a1ddb..168893d48413d 100644
--- a/llvm/test/CodeGen/X86/apx/nf-regressions.ll
+++ b/llvm/test/CodeGen/X86/apx/nf-regressions.ll
@@ -73,3 +73,36 @@ bb14:                                             ; preds = %bb12
 bb16:                                             ; preds = %bb14, %bb11, %bb10, %bb
   ret void
 }
+
+; Replacement of CMP should happen with SUB not with AND as it is AND_NF
+define void @cmp_peephole_and_nf(i64 %arg0, ptr %ptr1, ptr %ptr2) {
+; CHECK-LABEL: cmp_peephole_and_nf:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    negq %rdi
+; CHECK-NEXT:    movl %edi, %eax
+; CHECK-NEXT:    {nf} andl $1, %eax
+; CHECK-NEXT:    jb .LBB1_2
+; CHECK-NEXT:  # %bb.1: # %true
+; CHECK-NEXT:    testq %rax, %rax
+; CHECK-NEXT:    sete (%rsi)
+; CHECK-NEXT:    retq
+; CHECK-NEXT:  .LBB1_2: # %false
+; CHECK-NEXT:    movq %rdi, (%rsi)
+; CHECK-NEXT:    movq %rax, (%rdx)
+; CHECK-NEXT:    retq
+entry:
+  %sub_flag = sub i64 0, %arg0
+  %and_nf = and i64 %sub_flag, 1
+  %elim = icmp eq i64 0, %arg0
+  br i1 %elim, label %true, label %false
+
+true:
+  %8 = icmp eq i64 %and_nf, 0
+  store i1 %8, ptr %ptr1
+  ret void
+
+false:
+  store i64 %sub_flag, ptr %ptr1
+  store i64 %and_nf, ptr %ptr2
+  ret void
+}

[phoebewang]

LGTM, thanks!

[phoebewang]

SUB has NF variants too.

[e-kud]

Yes. I haven't liked this comment too because we don't see sub in the assembly. Reworded it with We must not try to replace CMP with AND_NF as it sets no flags. I also double checked the optimizeCompareInstr. In other cases we use switches where only CASE_ND is used. This one with isAND was an exception.

[RKSimon]

LGTM