AMDGPU: Make cvt_pknorm and cvt_pk intrinsics propagate poison #131059

arsenm · 2025-03-13T03:43:34Z

No description provided.

arsenm · 2025-03-13T03:43:49Z

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

llvmbot · 2025-03-13T03:44:09Z

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

Changes

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

2 Files Affected:

(modified) llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp (+3)
(modified) llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll (+96)

diff --git a/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp b/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
index bf53018439e9f..75b79b129d358 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
@@ -719,6 +719,9 @@ GCNTTIImpl::instCombineIntrinsic(InstCombiner &IC, IntrinsicInst &II) const {
     Value *Src0 = II.getArgOperand(0);
     Value *Src1 = II.getArgOperand(1);
 
+    if (isa<PoisonValue>(Src0) || isa<PoisonValue>(Src1))
+      return IC.replaceInstUsesWith(II, PoisonValue::get(II.getType()));
+
     if (isa<UndefValue>(Src0) && isa<UndefValue>(Src1)) {
       return IC.replaceInstUsesWith(II, UndefValue::get(II.getType()));
     }
diff --git a/llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll b/llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll
index 78606b1c869d1..14f31280cc878 100644
--- a/llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll
+++ b/llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll
@@ -1320,6 +1320,30 @@ define <2 x i16> @undef_cvt_pknorm_i16() {
   ret <2 x i16> %cvt
 }
 
+define <2 x i16> @poison_lhs_cvt_pknorm_i16(float %y) {
+; CHECK-LABEL: @poison_lhs_cvt_pknorm_i16(
+; CHECK-NEXT:    ret <2 x i16> poison
+;
+  %cvt = call <2 x i16> @llvm.amdgcn.cvt.pknorm.i16(float poison, float %y)
+  ret <2 x i16> %cvt
+}
+
+define <2 x i16> @poison_rhs_cvt_pknorm_i16(float %x) {
+; CHECK-LABEL: @poison_rhs_cvt_pknorm_i16(
+; CHECK-NEXT:    ret <2 x i16> poison
+;
+  %cvt = call <2 x i16> @llvm.amdgcn.cvt.pknorm.i16(float %x, float poison)
+  ret <2 x i16> %cvt
+}
+
+define <2 x i16> @poison_cvt_pknorm_i16() {
+; CHECK-LABEL: @poison_cvt_pknorm_i16(
+; CHECK-NEXT:    ret <2 x i16> poison
+;
+  %cvt = call <2 x i16> @llvm.amdgcn.cvt.pknorm.i16(float poison, float poison)
+  ret <2 x i16> %cvt
+}
+
 ; --------------------------------------------------------------------
 ; llvm.amdgcn.cvt.pknorm.u16
 ; --------------------------------------------------------------------
@@ -1352,6 +1376,30 @@ define <2 x i16> @undef_cvt_pknorm_u16() {
   ret <2 x i16> %cvt
 }
 
+define <2 x i16> @poison_lhs_cvt_pknorm_u16(float %y) {
+; CHECK-LABEL: @poison_lhs_cvt_pknorm_u16(
+; CHECK-NEXT:    ret <2 x i16> poison
+;
+  %cvt = call <2 x i16> @llvm.amdgcn.cvt.pknorm.u16(float poison, float %y)
+  ret <2 x i16> %cvt
+}
+
+define <2 x i16> @poison_rhs_cvt_pknorm_u16(float %x) {
+; CHECK-LABEL: @poison_rhs_cvt_pknorm_u16(
+; CHECK-NEXT:    ret <2 x i16> poison
+;
+  %cvt = call <2 x i16> @llvm.amdgcn.cvt.pknorm.u16(float %x, float poison)
+  ret <2 x i16> %cvt
+}
+
+define <2 x i16> @poison_cvt_pknorm_u16() {
+; CHECK-LABEL: @poison_cvt_pknorm_u16(
+; CHECK-NEXT:    ret <2 x i16> poison
+;
+  %cvt = call <2 x i16> @llvm.amdgcn.cvt.pknorm.u16(float poison, float poison)
+  ret <2 x i16> %cvt
+}
+
 ; --------------------------------------------------------------------
 ; llvm.amdgcn.cvt.pk.i16
 ; --------------------------------------------------------------------
@@ -1384,6 +1432,30 @@ define <2 x i16> @undef_cvt_pk_i16() {
   ret <2 x i16> %cvt
 }
 
+define <2 x i16> @poison_lhs_cvt_pk_i16(i32 %y) {
+; CHECK-LABEL: @poison_lhs_cvt_pk_i16(
+; CHECK-NEXT:    ret <2 x i16> poison
+;
+  %cvt = call <2 x i16> @llvm.amdgcn.cvt.pk.i16(i32 poison, i32 %y)
+  ret <2 x i16> %cvt
+}
+
+define <2 x i16> @poison_rhs_cvt_pk_i16(i32 %x) {
+; CHECK-LABEL: @poison_rhs_cvt_pk_i16(
+; CHECK-NEXT:    ret <2 x i16> poison
+;
+  %cvt = call <2 x i16> @llvm.amdgcn.cvt.pk.i16(i32 %x, i32 poison)
+  ret <2 x i16> %cvt
+}
+
+define <2 x i16> @poison_cvt_pk_i16() {
+; CHECK-LABEL: @poison_cvt_pk_i16(
+; CHECK-NEXT:    ret <2 x i16> poison
+;
+  %cvt = call <2 x i16> @llvm.amdgcn.cvt.pk.i16(i32 poison, i32 poison)
+  ret <2 x i16> %cvt
+}
+
 ; --------------------------------------------------------------------
 ; llvm.amdgcn.cvt.pk.u16
 ; --------------------------------------------------------------------
@@ -1416,6 +1488,30 @@ define <2 x i16> @undef_cvt_pk_u16() {
   ret <2 x i16> %cvt
 }
 
+define <2 x i16> @poison_lhs_cvt_pk_u16(i32 %y) {
+; CHECK-LABEL: @poison_lhs_cvt_pk_u16(
+; CHECK-NEXT:    ret <2 x i16> poison
+;
+  %cvt = call <2 x i16> @llvm.amdgcn.cvt.pk.u16(i32 poison, i32 %y)
+  ret <2 x i16> %cvt
+}
+
+define <2 x i16> @poison_rhs_cvt_pk_u16(i32 %x) {
+; CHECK-LABEL: @poison_rhs_cvt_pk_u16(
+; CHECK-NEXT:    ret <2 x i16> poison
+;
+  %cvt = call <2 x i16> @llvm.amdgcn.cvt.pk.u16(i32 %x, i32 poison)
+  ret <2 x i16> %cvt
+}
+
+define <2 x i16> @poison_cvt_pk_u16() {
+; CHECK-LABEL: @poison_cvt_pk_u16(
+; CHECK-NEXT:    ret <2 x i16> poison
+;
+  %cvt = call <2 x i16> @llvm.amdgcn.cvt.pk.u16(i32 poison, i32 poison)
+  ret <2 x i16> %cvt
+}
+
 ; --------------------------------------------------------------------
 ; llvm.amdgcn.ubfe
 ; --------------------------------------------------------------------

pravinjagtap

LGTM

arsenm · 2025-03-13T06:09:40Z

llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll

+; CHECK-NEXT:    ret <2 x i16> poison
+;
+  %cvt = call <2 x i16> @llvm.amdgcn.cvt.pknorm.i16(float poison, float %y)
+  ret <2 x i16> %cvt


Maybe we should be only propagating poison for the one element. That's how the IR casts would behave

But that's poison propagation by element in a single input. This is scattered across arguments. The equivalent generic IR would still be poison

Actually not: https://godbolt.org/z/qnPeqdaMT

arsenm · 2025-03-14T04:23:32Z

Merge activity

Mar 14, 12:23 AM EDT: A user started a stack merge that includes this pull request via Graphite.
Mar 14, 12:25 AM EDT: Graphite rebased this pull request as part of a merge.
Mar 14, 12:27 AM EDT: A user merged this pull request with Graphite.

…131059)

arsenm added the backend:AMDGPU label Mar 13, 2025 — with Graphite App

This was referenced Mar 13, 2025

AMDGPU: Make fmed3 intrinsic propagate poison #131060

Merged

AMDGPU: Make ballot intrinsic propagate poison #131061

Merged

arsenm requested review from jayfoad, Pierre-vh, pravinjagtap, rampitec and shiltian March 13, 2025 03:44

arsenm marked this pull request as ready for review March 13, 2025 03:44

llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Mar 13, 2025

pravinjagtap approved these changes Mar 13, 2025

View reviewed changes

This was referenced Mar 13, 2025

AMDGPU: Make fmul_legacy intrinsic propagate poison #131062

Merged

AMDGPU: Make fma_legacy intrinsic propagate poison #131063

Merged

arsenm commented Mar 13, 2025

View reviewed changes

arsenm force-pushed the users/arsenm/amdgpu/propagate-poison-cvt-pk-intrinsics branch from c2fad37 to bd6f2f2 Compare March 14, 2025 01:21

arsenm added 2 commits March 14, 2025 04:25

AMDGPU: Make cvt_pknorm and cvt_pk intrinsics propagate poison

6832d86

Only propagate if both inputs poison

d17bc7f

arsenm force-pushed the users/arsenm/amdgpu/propagate-poison-cvt-pk-intrinsics branch from bd6f2f2 to d17bc7f Compare March 14, 2025 04:25

arsenm merged commit 9b887f5 into main Mar 14, 2025
6 of 10 checks passed

arsenm deleted the users/arsenm/amdgpu/propagate-poison-cvt-pk-intrinsics branch March 14, 2025 04:27

frederik-h pushed a commit to frederik-h/llvm-project that referenced this pull request Mar 18, 2025

AMDGPU: Make cvt_pknorm and cvt_pk intrinsics propagate poison (llvm#…

671862e

…131059)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

AMDGPU: Make cvt_pknorm and cvt_pk intrinsics propagate poison #131059

AMDGPU: Make cvt_pknorm and cvt_pk intrinsics propagate poison #131059

Uh oh!

arsenm commented Mar 13, 2025

Uh oh!

arsenm commented Mar 13, 2025 •

edited

Loading

Uh oh!

llvmbot commented Mar 13, 2025 •

edited

Loading

Uh oh!

pravinjagtap left a comment

Uh oh!

arsenm Mar 13, 2025

Uh oh!

arsenm Mar 13, 2025

Uh oh!

arsenm Mar 13, 2025

Uh oh!

arsenm commented Mar 14, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

AMDGPU: Make cvt_pknorm and cvt_pk intrinsics propagate poison #131059

AMDGPU: Make cvt_pknorm and cvt_pk intrinsics propagate poison #131059

Uh oh!

Conversation

arsenm commented Mar 13, 2025

Uh oh!

arsenm commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pravinjagtap left a comment

Choose a reason for hiding this comment

Uh oh!

arsenm Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

arsenm Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

arsenm Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

arsenm commented Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Uh oh!

arsenm commented Mar 13, 2025 •

edited

Loading

llvmbot commented Mar 13, 2025 •

edited

Loading

arsenm commented Mar 14, 2025 •

edited

Loading