[PATCH] D144984: Fix `(shift X, (xor Y, N-1))` -> `(shift X, (not Y))` by properly inserting `not Y` into DAG. [#61038]

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n created this revision.
Herald added subscribers: pengfei, hiraditya.
Herald added a project: All.
goldstein.w.n requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Previously not inserting the `-1` in `not Y` (`xor Y, -1`) into the
DAG. Not inserting `-1` as a DAG node comes up as a bug when doing
`(xor (shl 1, A), B)` -> `(btc A, B)`. `btc` requires `B` (dst) to be
a register.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144984

Files:
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/test/CodeGen/X86/pr61038.ll


Index: llvm/test/CodeGen/X86/pr61038.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/X86/pr61038.ll
@@ -0,0 +1,64 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mcpu=skylake-avx512 | FileCheck %s --check-prefix=CHECK,CHECK-X64-SKX
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mcpu=x86-64-v4 | FileCheck %s --check-prefix=CHECK,CHECK-X64-V4
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+bmi | FileCheck %s --check-prefix=CHECK,CHECK-X64-BMI
+
+declare i32 @llvm.cttz.i32(i32, i1 immarg)
+define void @test_61038(ptr %tmp_buffer) {
+; CHECK-X64-SKX-LABEL: test_61038:
+; CHECK-X64-SKX:       # %bb.0: # %entry
+; CHECK-X64-SKX-NEXT:    tzcntl %eax, %eax
+; CHECK-X64-SKX-NEXT:    movabsq $8589934591, %rcx # imm = 0x1FFFFFFFF
+; CHECK-X64-SKX-NEXT:    movq $-1, %rdx
+; CHECK-X64-SKX-NEXT:    btcq %rax, %rdx
+; CHECK-X64-SKX-NEXT:    shrxq %rdx, %rcx, %rdx
+; CHECK-X64-SKX-NEXT:    btcq %rax, %rcx
+; CHECK-X64-SKX-NEXT:    xorl %eax, %eax
+; CHECK-X64-SKX-NEXT:    cmpq $64, %rcx
+; CHECK-X64-SKX-NEXT:    cmovael %eax, %edx
+; CHECK-X64-SKX-NEXT:    movl %edx, (%rdi)
+; CHECK-X64-SKX-NEXT:    retq
+;
+; CHECK-X64-V4-LABEL: test_61038:
+; CHECK-X64-V4:       # %bb.0: # %entry
+; CHECK-X64-V4-NEXT:    tzcntl %eax, %eax
+; CHECK-X64-V4-NEXT:    movabsq $8589934591, %rcx # imm = 0x1FFFFFFFF
+; CHECK-X64-V4-NEXT:    movq $-1, %rdx
+; CHECK-X64-V4-NEXT:    btcq %rax, %rdx
+; CHECK-X64-V4-NEXT:    shrxq %rdx, %rcx, %rdx
+; CHECK-X64-V4-NEXT:    btcq %rax, %rcx
+; CHECK-X64-V4-NEXT:    xorl %eax, %eax
+; CHECK-X64-V4-NEXT:    cmpq $64, %rcx
+; CHECK-X64-V4-NEXT:    cmovael %eax, %edx
+; CHECK-X64-V4-NEXT:    movl %edx, (%rdi)
+; CHECK-X64-V4-NEXT:    retq
+;
+; CHECK-X64-BMI-LABEL: test_61038:
+; CHECK-X64-BMI:       # %bb.0: # %entry
+; CHECK-X64-BMI-NEXT:    tzcntl %eax, %eax
+; CHECK-X64-BMI-NEXT:    movabsq $8589934591, %rdx # imm = 0x1FFFFFFFF
+; CHECK-X64-BMI-NEXT:    movq %rdx, %rsi
+; CHECK-X64-BMI-NEXT:    btcq %rax, %rsi
+; CHECK-X64-BMI-NEXT:    movq $-1, %rcx
+; CHECK-X64-BMI-NEXT:    btcq %rax, %rcx
+; CHECK-X64-BMI-NEXT:    # kill: def $cl killed $cl killed $rcx
+; CHECK-X64-BMI-NEXT:    shrq %cl, %rdx
+; CHECK-X64-BMI-NEXT:    xorl %eax, %eax
+; CHECK-X64-BMI-NEXT:    cmpq $64, %rsi
+; CHECK-X64-BMI-NEXT:    cmovael %eax, %edx
+; CHECK-X64-BMI-NEXT:    movl %edx, (%rdi)
+; CHECK-X64-BMI-NEXT:    retq
+entry:
+  %0 = tail call i32 @llvm.cttz.i32(i32 poison, i1 false)
+  %1 = zext i32 %0 to i64
+  %2 = shl nuw nsw i64 1, %1
+  %3 = xor i64 %2, 8589934591
+  %4 = icmp ugt i64 %3, 63
+  %x11.op.i = lshr i64 8589934591, %3
+  %5 = trunc i64 %x11.op.i to i32
+  %6 = select i1 %4, i32 0, i32 %5
+  store i32 %6, ptr %tmp_buffer, align 4
+  ret void
+}
+;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
+; CHECK: {{.*}}
Index: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
===================================================================
--- llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
+++ llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
@@ -4021,7 +4021,10 @@
 
       EVT OpVT = ShiftAmt.getValueType();
 
-      NewShiftAmt = CurDAG->getNOT(DL, Add0C == nullptr ? Add0 : Add1, OpVT);
+      SDValue AllOnes = CurDAG->getAllOnesConstant(DL, OpVT);
+      NewShiftAmt = CurDAG->getNode(ISD::XOR, DL, OpVT,
+                                    Add0C == nullptr ? Add0 : Add1, AllOnes);
+      insertDAGNode(*CurDAG, OrigShiftAmt, AllOnes);
       insertDAGNode(*CurDAG, OrigShiftAmt, NewShiftAmt);
       // If we are shifting by N-X where N == 0 mod Size, then just shift by
       // -X to generate a NEG instead of a SUB of a constant.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144984.501201.patch
Type: text/x-patch
Size: 3767 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230228/863ff724/attachment.bin>