Skip to content

Conversation

RKSimon
Copy link
Collaborator

@RKSimon RKSimon commented Sep 5, 2025

@llvmbot llvmbot added backend:AArch64 llvm:SelectionDAG SelectionDAGISel as well labels Sep 5, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 5, 2025

@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-llvm-selectiondag

Author: Simon Pilgrim (RKSimon)

Changes

AVGFLOORU: https://alive2.llvm.org/ce/z/4pfi4i
AVGCEILU: https://alive2.llvm.org/ce/z/CGvWiA

The signed variants are still proving annoying to get test cases to work properly

Fixes half of #147696


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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+2)
  • (modified) llvm/test/CodeGen/AArch64/freeze.ll (-6)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 9668d253d52ae..63a9f8bc2c615 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -5670,6 +5670,8 @@ bool SelectionDAG::canCreateUndefOrPoison(SDValue Op, const APInt &DemandedElts,
   case ISD::UADDSAT:
   case ISD::SSUBSAT:
   case ISD::USUBSAT:
+  case ISD::AVGFLOORU:
+  case ISD::AVGCEILU:
   case ISD::MULHU:
   case ISD::MULHS:
   case ISD::ABDU:
diff --git a/llvm/test/CodeGen/AArch64/freeze.ll b/llvm/test/CodeGen/AArch64/freeze.ll
index 2a33a4c061dce..7a9d6b7e52457 100644
--- a/llvm/test/CodeGen/AArch64/freeze.ll
+++ b/llvm/test/CodeGen/AArch64/freeze.ll
@@ -430,16 +430,13 @@ define <8 x i16> @freeze_abds(<8 x i16> %a, <8 x i16> %b) {
   ret <8 x i16> %r
 }
 
-; TODO: Unnecessary final and
 define <8 x i16> @freeze_uhadd(<8 x i16> %a0, <8 x i16> %a1) {
 ; CHECK-LABEL: freeze_uhadd:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    movi v2.8h, #15
 ; CHECK-NEXT:    and v0.16b, v0.16b, v2.16b
 ; CHECK-NEXT:    and v1.16b, v1.16b, v2.16b
-; CHECK-NEXT:    movi v2.8h, #31
 ; CHECK-NEXT:    uhadd v0.8h, v0.8h, v1.8h
-; CHECK-NEXT:    and v0.16b, v0.16b, v2.16b
 ; CHECK-NEXT:    ret
   %m0 = and <8 x i16> %a0, splat (i16 15)
   %m1 = and <8 x i16> %a1, splat (i16 15)
@@ -449,16 +446,13 @@ define <8 x i16> @freeze_uhadd(<8 x i16> %a0, <8 x i16> %a1) {
   ret <8 x i16> %masked
 }
 
-; TODO: Unnecessary final and
 define <8 x i16> @freeze_urhadd(<8 x i16> %a0, <8 x i16> %a1) {
 ; CHECK-LABEL: freeze_urhadd:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    movi v2.8h, #15
 ; CHECK-NEXT:    and v0.16b, v0.16b, v2.16b
 ; CHECK-NEXT:    and v1.16b, v1.16b, v2.16b
-; CHECK-NEXT:    movi v2.8h, #31
 ; CHECK-NEXT:    urhadd v0.8h, v0.8h, v1.8h
-; CHECK-NEXT:    and v0.16b, v0.16b, v2.16b
 ; CHECK-NEXT:    ret
   %m0 = and <8 x i16> %a0, splat (i16 15)
   %m1 = and <8 x i16> %a1, splat (i16 15)

@llvmbot
Copy link
Member

llvmbot commented Sep 5, 2025

@llvm/pr-subscribers-backend-aarch64

Author: Simon Pilgrim (RKSimon)

Changes

AVGFLOORU: https://alive2.llvm.org/ce/z/4pfi4i
AVGCEILU: https://alive2.llvm.org/ce/z/CGvWiA

The signed variants are still proving annoying to get test cases to work properly

Fixes half of #147696


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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+2)
  • (modified) llvm/test/CodeGen/AArch64/freeze.ll (-6)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 9668d253d52ae..63a9f8bc2c615 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -5670,6 +5670,8 @@ bool SelectionDAG::canCreateUndefOrPoison(SDValue Op, const APInt &DemandedElts,
   case ISD::UADDSAT:
   case ISD::SSUBSAT:
   case ISD::USUBSAT:
+  case ISD::AVGFLOORU:
+  case ISD::AVGCEILU:
   case ISD::MULHU:
   case ISD::MULHS:
   case ISD::ABDU:
diff --git a/llvm/test/CodeGen/AArch64/freeze.ll b/llvm/test/CodeGen/AArch64/freeze.ll
index 2a33a4c061dce..7a9d6b7e52457 100644
--- a/llvm/test/CodeGen/AArch64/freeze.ll
+++ b/llvm/test/CodeGen/AArch64/freeze.ll
@@ -430,16 +430,13 @@ define <8 x i16> @freeze_abds(<8 x i16> %a, <8 x i16> %b) {
   ret <8 x i16> %r
 }
 
-; TODO: Unnecessary final and
 define <8 x i16> @freeze_uhadd(<8 x i16> %a0, <8 x i16> %a1) {
 ; CHECK-LABEL: freeze_uhadd:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    movi v2.8h, #15
 ; CHECK-NEXT:    and v0.16b, v0.16b, v2.16b
 ; CHECK-NEXT:    and v1.16b, v1.16b, v2.16b
-; CHECK-NEXT:    movi v2.8h, #31
 ; CHECK-NEXT:    uhadd v0.8h, v0.8h, v1.8h
-; CHECK-NEXT:    and v0.16b, v0.16b, v2.16b
 ; CHECK-NEXT:    ret
   %m0 = and <8 x i16> %a0, splat (i16 15)
   %m1 = and <8 x i16> %a1, splat (i16 15)
@@ -449,16 +446,13 @@ define <8 x i16> @freeze_uhadd(<8 x i16> %a0, <8 x i16> %a1) {
   ret <8 x i16> %masked
 }
 
-; TODO: Unnecessary final and
 define <8 x i16> @freeze_urhadd(<8 x i16> %a0, <8 x i16> %a1) {
 ; CHECK-LABEL: freeze_urhadd:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    movi v2.8h, #15
 ; CHECK-NEXT:    and v0.16b, v0.16b, v2.16b
 ; CHECK-NEXT:    and v1.16b, v1.16b, v2.16b
-; CHECK-NEXT:    movi v2.8h, #31
 ; CHECK-NEXT:    urhadd v0.8h, v0.8h, v1.8h
-; CHECK-NEXT:    and v0.16b, v0.16b, v2.16b
 ; CHECK-NEXT:    ret
   %m0 = and <8 x i16> %a0, splat (i16 15)
   %m1 = and <8 x i16> %a1, splat (i16 15)

… create undef/poison

AVGFLOORU: https://alive2.llvm.org/ce/z/4pfi4i
AVGCEILU: https://alive2.llvm.org/ce/z/CGvWiA

The signed variants are still proving annoying to get test cases to work properly

Fixes half of llvm#147696
Copy link
Contributor

@jayfoad jayfoad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is intuitively obvious to me that AVGFLOORU/AVGCEILU/AVGFLOORS/AVGCEILS have a well defined result for all (non-undef non-poison) inputs, so I think this is fine even without Alive2 proofs.

@RKSimon
Copy link
Collaborator Author

RKSimon commented Sep 5, 2025

It is intuitively obvious to me that AVGFLOORU/AVGCEILU/AVGFLOORS/AVGCEILS have a well defined result for all (non-undef non-poison) inputs, so I think this is fine even without Alive2 proofs.

Are you happy if I add AVGFLOORS/AVGCEILS handling top this patch as well?

@jayfoad
Copy link
Contributor

jayfoad commented Sep 5, 2025

It is intuitively obvious to me that AVGFLOORU/AVGCEILU/AVGFLOORS/AVGCEILS have a well defined result for all (non-undef non-poison) inputs, so I think this is fine even without Alive2 proofs.

Are you happy if I add AVGFLOORS/AVGCEILS handling top this patch as well?

Yes.

@RKSimon RKSimon enabled auto-merge (squash) September 5, 2025 10:44
@RKSimon RKSimon changed the title [DAG] SelectionDAG::canCreateUndefOrPoison - AVGFLOORU/AVGCEILU don't create undef/poison [DAG] SelectionDAG::canCreateUndefOrPoison - AVGFLOOR/AVGCEIL don't create undef/poison Sep 5, 2025
@RKSimon RKSimon disabled auto-merge September 5, 2025 10:47
@RKSimon RKSimon enabled auto-merge (squash) September 5, 2025 10:47
@RKSimon RKSimon merged commit 98f1e4a into llvm:main Sep 5, 2025
9 checks passed
@RKSimon RKSimon deleted the dag-avgu-nopoison branch September 5, 2025 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DAG] SelectionDAG::canCreateUndefOrPoison - add ISD::AVGFLOORS/AVGFLOORU/AVGCEILS/AVGCEILU handling + tests
3 participants