DAG: Call SimplifyDemandedBits on copysign value operand #97180

arsenm · 2024-06-29T20:24:55Z

So far the only cases that seem to benefit are the weird
copysign with different typed inputs.

arsenm · 2024-06-29T20:25:12Z

DAG: Call SimplifyDemandedBits on copysign value operand #97180 👈
DAG: Call SimplifyDemandedBits on fcopysign sign value #97151 : 1 other dependent PR (#97152 )
AMDGPU: Add baseline test for copysign combine #97150
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @arsenm and the rest of your teammates on Graphite

llvmbot · 2024-06-29T20:25:22Z

@llvm/pr-subscribers-backend-powerpc
@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

Changes

So far the only cases that seem to benefit are the weird
copysign with different typed inputs.

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

2 Files Affected:

(modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+5)
(modified) llvm/test/CodeGen/AMDGPU/fcopysign.f16.ll (+15-25)

diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index eaab2b3421190..5d72b5e1e30dd 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -17571,6 +17571,11 @@ SDValue DAGCombiner::visitFCOPYSIGN(SDNode *N) {
                            APInt::getSignMask(SignVT.getScalarSizeInBits())))
     return SDValue(N, 0);
 
+  // We only take the non-sign bits from the value operand
+  if (SimplifyDemandedBits(N0,
+                           APInt::getSignedMaxValue(VT.getScalarSizeInBits())))
+    return SDValue(N, 0);
+
   return SDValue();
 }
 
diff --git a/llvm/test/CodeGen/AMDGPU/fcopysign.f16.ll b/llvm/test/CodeGen/AMDGPU/fcopysign.f16.ll
index b8936911f0576..eda1709e4fd59 100644
--- a/llvm/test/CodeGen/AMDGPU/fcopysign.f16.ll
+++ b/llvm/test/CodeGen/AMDGPU/fcopysign.f16.ll
@@ -1639,10 +1639,7 @@ define amdgpu_kernel void @s_copysign_out_f16_mag_f64_sign_f16(ptr addrspace(1)
 ; VI-NEXT:    v_or_b32_e32 v2, 0x7c00, v2
 ; VI-NEXT:    v_mov_b32_e32 v3, s2
 ; VI-NEXT:    s_cselect_b64 vcc, -1, 0
-; VI-NEXT:    s_lshr_b32 s0, s7, 16
 ; VI-NEXT:    v_cndmask_b32_e32 v2, v3, v2, vcc
-; VI-NEXT:    s_and_b32 s0, s0, 0x8000
-; VI-NEXT:    v_or_b32_e32 v2, s0, v2
 ; VI-NEXT:    s_movk_i32 s0, 0x7fff
 ; VI-NEXT:    v_mov_b32_e32 v3, s8
 ; VI-NEXT:    v_bfi_b32 v2, s0, v2, v3
@@ -1673,36 +1670,33 @@ define amdgpu_kernel void @s_copysign_out_f16_mag_f64_sign_f16(ptr addrspace(1)
 ; GFX9-NEXT:    v_lshlrev_b32_e64 v1, v1, s2
 ; GFX9-NEXT:    v_cmp_ne_u32_e32 vcc, s0, v1
 ; GFX9-NEXT:    v_cndmask_b32_e64 v1, 0, 1, vcc
-; GFX9-NEXT:    s_add_i32 s9, s1, 0xfffffc10
+; GFX9-NEXT:    s_add_i32 s7, s1, 0xfffffc10
 ; GFX9-NEXT:    v_readfirstlane_b32 s0, v1
-; GFX9-NEXT:    s_lshl_b32 s1, s9, 12
+; GFX9-NEXT:    s_lshl_b32 s1, s7, 12
 ; GFX9-NEXT:    s_or_b32 s0, s2, s0
 ; GFX9-NEXT:    s_or_b32 s1, s6, s1
-; GFX9-NEXT:    s_cmp_lt_i32 s9, 1
-; GFX9-NEXT:    s_cselect_b32 s10, s0, s1
-; GFX9-NEXT:    s_and_b32 s2, s10, 7
+; GFX9-NEXT:    s_cmp_lt_i32 s7, 1
+; GFX9-NEXT:    s_cselect_b32 s9, s0, s1
+; GFX9-NEXT:    s_and_b32 s2, s9, 7
 ; GFX9-NEXT:    s_cmp_gt_i32 s2, 5
 ; GFX9-NEXT:    s_cselect_b64 s[0:1], -1, 0
 ; GFX9-NEXT:    s_cmp_eq_u32 s2, 3
 ; GFX9-NEXT:    s_cselect_b64 s[2:3], -1, 0
 ; GFX9-NEXT:    s_or_b64 s[0:1], s[2:3], s[0:1]
-; GFX9-NEXT:    s_lshr_b32 s2, s10, 2
+; GFX9-NEXT:    s_lshr_b32 s2, s9, 2
 ; GFX9-NEXT:    s_cmp_lg_u64 s[0:1], 0
 ; GFX9-NEXT:    s_addc_u32 s0, s2, 0
-; GFX9-NEXT:    s_cmp_lt_i32 s9, 31
+; GFX9-NEXT:    s_cmp_lt_i32 s7, 31
 ; GFX9-NEXT:    s_cselect_b32 s2, s0, 0x7c00
 ; GFX9-NEXT:    s_cmp_lg_u32 s6, 0
 ; GFX9-NEXT:    s_cselect_b64 s[0:1], -1, 0
 ; GFX9-NEXT:    v_cndmask_b32_e64 v1, 0, 1, s[0:1]
 ; GFX9-NEXT:    v_lshlrev_b32_e32 v1, 9, v1
-; GFX9-NEXT:    s_cmpk_eq_i32 s9, 0x40f
+; GFX9-NEXT:    s_cmpk_eq_i32 s7, 0x40f
 ; GFX9-NEXT:    v_or_b32_e32 v1, 0x7c00, v1
 ; GFX9-NEXT:    v_mov_b32_e32 v2, s2
 ; GFX9-NEXT:    s_cselect_b64 vcc, -1, 0
-; GFX9-NEXT:    s_lshr_b32 s0, s7, 16
 ; GFX9-NEXT:    v_cndmask_b32_e32 v1, v2, v1, vcc
-; GFX9-NEXT:    s_and_b32 s0, s0, 0x8000
-; GFX9-NEXT:    v_or_b32_e32 v1, s0, v1
 ; GFX9-NEXT:    s_movk_i32 s0, 0x7fff
 ; GFX9-NEXT:    v_mov_b32_e32 v2, s8
 ; GFX9-NEXT:    v_bfi_b32 v1, s0, v1, v2
@@ -1728,13 +1722,13 @@ define amdgpu_kernel void @s_copysign_out_f16_mag_f64_sign_f16(ptr addrspace(1)
 ; GFX11-NEXT:    s_addk_i32 s1, 0xfc10
 ; GFX11-NEXT:    v_med3_i32 v1, s3, 0, 13
 ; GFX11-NEXT:    v_readfirstlane_b32 s3, v0
-; GFX11-NEXT:    s_lshl_b32 s8, s1, 12
+; GFX11-NEXT:    s_lshl_b32 s7, s1, 12
 ; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_2) | instskip(NEXT) | instid1(VALU_DEP_2)
 ; GFX11-NEXT:    v_readfirstlane_b32 s6, v1
 ; GFX11-NEXT:    s_or_b32 s2, s2, s3
 ; GFX11-NEXT:    s_delay_alu instid0(SALU_CYCLE_1) | instskip(SKIP_1) | instid1(VALU_DEP_1)
 ; GFX11-NEXT:    s_or_b32 s3, s2, 0x1000
-; GFX11-NEXT:    s_or_b32 s8, s2, s8
+; GFX11-NEXT:    s_or_b32 s7, s2, s7
 ; GFX11-NEXT:    s_lshr_b32 s6, s3, s6
 ; GFX11-NEXT:    s_delay_alu instid0(SALU_CYCLE_1) | instskip(SKIP_1) | instid1(VALU_DEP_2)
 ; GFX11-NEXT:    v_lshlrev_b32_e64 v0, v1, s6
@@ -1745,15 +1739,15 @@ define amdgpu_kernel void @s_copysign_out_f16_mag_f64_sign_f16(ptr addrspace(1)
 ; GFX11-NEXT:    v_readfirstlane_b32 s3, v0
 ; GFX11-NEXT:    s_or_b32 s3, s6, s3
 ; GFX11-NEXT:    s_cmp_lt_i32 s1, 1
-; GFX11-NEXT:    s_cselect_b32 s3, s3, s8
+; GFX11-NEXT:    s_cselect_b32 s3, s3, s7
 ; GFX11-NEXT:    s_delay_alu instid0(SALU_CYCLE_1) | instskip(NEXT) | instid1(SALU_CYCLE_1)
 ; GFX11-NEXT:    s_and_b32 s6, s3, 7
 ; GFX11-NEXT:    s_cmp_gt_i32 s6, 5
-; GFX11-NEXT:    s_cselect_b32 s8, -1, 0
+; GFX11-NEXT:    s_cselect_b32 s7, -1, 0
 ; GFX11-NEXT:    s_cmp_eq_u32 s6, 3
 ; GFX11-NEXT:    s_cselect_b32 s6, -1, 0
 ; GFX11-NEXT:    s_lshr_b32 s3, s3, 2
-; GFX11-NEXT:    s_or_b32 s6, s6, s8
+; GFX11-NEXT:    s_or_b32 s6, s6, s7
 ; GFX11-NEXT:    s_delay_alu instid0(SALU_CYCLE_1)
 ; GFX11-NEXT:    s_cmp_lg_u32 s6, 0
 ; GFX11-NEXT:    s_addc_u32 s3, s3, 0
@@ -1764,15 +1758,11 @@ define amdgpu_kernel void @s_copysign_out_f16_mag_f64_sign_f16(ptr addrspace(1)
 ; GFX11-NEXT:    s_cmpk_eq_i32 s1, 0x40f
 ; GFX11-NEXT:    v_cndmask_b32_e64 v0, 0, 1, s2
 ; GFX11-NEXT:    s_cselect_b32 vcc_lo, -1, 0
-; GFX11-NEXT:    s_lshr_b32 s1, s7, 16
-; GFX11-NEXT:    s_delay_alu instid0(SALU_CYCLE_1) | instskip(NEXT) | instid1(VALU_DEP_1)
-; GFX11-NEXT:    s_and_b32 s1, s1, 0x8000
-; GFX11-NEXT:    v_lshlrev_b32_e32 v0, 9, v0
 ; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_1) | instskip(NEXT) | instid1(VALU_DEP_1)
+; GFX11-NEXT:    v_lshlrev_b32_e32 v0, 9, v0
 ; GFX11-NEXT:    v_or_b32_e32 v0, 0x7c00, v0
-; GFX11-NEXT:    v_cndmask_b32_e32 v0, s3, v0, vcc_lo
 ; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_1) | instskip(NEXT) | instid1(VALU_DEP_1)
-; GFX11-NEXT:    v_or_b32_e32 v0, s1, v0
+; GFX11-NEXT:    v_cndmask_b32_e32 v0, s3, v0, vcc_lo
 ; GFX11-NEXT:    v_bfi_b32 v0, 0x7fff, v0, s0
 ; GFX11-NEXT:    global_store_b16 v1, v0, s[4:5]
 ; GFX11-NEXT:    s_nop 0

RKSimon

LGTM

arsenm · 2024-07-01T10:08:23Z

Merge activity

Jul 1, 6:08 AM EDT: @arsenm started a stack merge that includes this pull request via Graphite.
Jul 1, 6:22 AM EDT: Graphite rebased this pull request as part of a merge.
Jul 1, 6:26 AM EDT: Graphite rebased this pull request as part of a merge.
Jul 1, 6:29 AM EDT: @arsenm merged this pull request with Graphite.

So far the only cases that seem to benefit are the weird copysign with different typed inputs.

jayfoad · 2024-07-01T10:39:51Z

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

@@ -17571,6 +17571,11 @@ SDValue DAGCombiner::visitFCOPYSIGN(SDNode *N) {
                           APInt::getSignMask(SignVT.getScalarSizeInBits())))
    return SDValue(N, 0);

+  // We only take the non-sign bits from the value operand
+  if (SimplifyDemandedBits(N0,


This looks like it could subsume all these optimizations from above:

// copysign(fabs(x), y) -> copysign(x, y) // copysign(fneg(x), y) -> copysign(x, y) // copysign(copysign(x,z), y) -> copysign(x, y)

Probably, but SimplifyDemandedBits doesn't handle fabs, fneg or copysign now

So far the only cases that seem to benefit are the weird copysign with different typed inputs.

arsenm requested review from andykaylor, bogner, jayfoad, jcranmer-intel, RKSimon and topperc June 29, 2024 20:25

arsenm added backend:AMDGPU backend:PowerPC floating-point Floating-point math llvm:SelectionDAG SelectionDAGISel as well labels Jun 29, 2024 — with Graphite App

arsenm marked this pull request as ready for review June 29, 2024 20:25

This was referenced Jun 29, 2024

AMDGPU: Add baseline test for copysign combine #97150

Merged

DAG: Call SimplifyDemandedBits on fcopysign sign value #97151

Merged

arsenm mentioned this pull request Jun 29, 2024

AMDGPU: Use real copysign in fast pow #97152

Merged

RKSimon approved these changes Jul 1, 2024

View reviewed changes

arsenm force-pushed the users/arsenm/dag-simplifydemanded-bits-copysign-sign-operand branch from 1cc5dca to 627e1f1 Compare July 1, 2024 10:15

Base automatically changed from users/arsenm/dag-simplifydemanded-bits-copysign-sign-operand to main July 1, 2024 10:19

arsenm force-pushed the users/arsenm/dag-simplifydemandedbits-copysign-value-operand branch from a064f83 to 18e1c04 Compare July 1, 2024 10:22

DAG: Call SimplifyDemandedBits on copysign value operand

9fae10f

So far the only cases that seem to benefit are the weird copysign with different typed inputs.

arsenm force-pushed the users/arsenm/dag-simplifydemandedbits-copysign-value-operand branch from 18e1c04 to 9fae10f Compare July 1, 2024 10:25

arsenm merged commit 8eee6d3 into main Jul 1, 2024
4 of 6 checks passed

arsenm deleted the users/arsenm/dag-simplifydemandedbits-copysign-value-operand branch July 1, 2024 10:29

jayfoad reviewed Jul 1, 2024

View reviewed changes

arsenm mentioned this pull request Jul 2, 2024

SimplifyDemandedBits should handle fneg/fabs/fcopysign #97427

Open

lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024

DAG: Call SimplifyDemandedBits on copysign value operand (llvm#97180)

7409895

So far the only cases that seem to benefit are the weird copysign with different typed inputs.

kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024

DAG: Call SimplifyDemandedBits on copysign value operand (llvm#97180)

98ab96a

So far the only cases that seem to benefit are the weird copysign with different typed inputs.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DAG: Call SimplifyDemandedBits on copysign value operand #97180

DAG: Call SimplifyDemandedBits on copysign value operand #97180

arsenm commented Jun 29, 2024

arsenm commented Jun 29, 2024 •

edited

Loading

llvmbot commented Jun 29, 2024 •

edited

Loading

RKSimon left a comment

arsenm commented Jul 1, 2024 •

edited

Loading

jayfoad Jul 1, 2024

arsenm Jul 2, 2024

DAG: Call SimplifyDemandedBits on copysign value operand #97180

DAG: Call SimplifyDemandedBits on copysign value operand #97180

Conversation

arsenm commented Jun 29, 2024

arsenm commented Jun 29, 2024 • edited Loading

llvmbot commented Jun 29, 2024 • edited Loading

RKSimon left a comment

Choose a reason for hiding this comment

arsenm commented Jul 1, 2024 • edited Loading

Merge activity

jayfoad Jul 1, 2024

Choose a reason for hiding this comment

arsenm Jul 2, 2024

Choose a reason for hiding this comment

arsenm commented Jun 29, 2024 •

edited

Loading

llvmbot commented Jun 29, 2024 •

edited

Loading

arsenm commented Jul 1, 2024 •

edited

Loading