Skip to content

[AMDGPU] Support arbitrary types in amdgcn.dead #134841

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 5, 2025
Merged

[AMDGPU] Support arbitrary types in amdgcn.dead #134841

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9971bf8
[AMDGPU] Support arbitrary types in amdgcn.dead
rovka Apr 3, 2025
bbc9f77
Address review comments
rovka Apr 23, 2025
fbc9974
Add test with array in struct
rovka Apr 23, 2025
888fbf0
Merge branch 'main' into dead-other-types
rovka Apr 30, 2025
d791af5
Use range-based for
rovka May 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1191,12 +1191,6 @@ bool AMDGPUInstructionSelector::selectG_INTRINSIC(MachineInstr &I) const {
case Intrinsic::amdgcn_permlane16_swap:
case Intrinsic::amdgcn_permlane32_swap:
return selectPermlaneSwapIntrin(I, IntrinsicID);
case Intrinsic::amdgcn_dead: {
I.setDesc(TII.get(TargetOpcode::IMPLICIT_DEF));
I.removeOperand(1); // drop intrinsic ID
return RBI.constrainGenericRegister(I.getOperand(0).getReg(),
AMDGPU::VGPR_32RegClass, *MRI);
}
default:
return selectImpl(I, *CoverageInfo);
}
Expand Down
7 changes: 7 additions & 0 deletions llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7658,6 +7658,13 @@ bool AMDGPULegalizerInfo::legalizeIntrinsic(LegalizerHelper &Helper,
return legalizeLaneOp(Helper, MI, IntrID);
case Intrinsic::amdgcn_s_buffer_prefetch_data:
return legalizeSBufferPrefetch(Helper, MI);
case Intrinsic::amdgcn_dead: {
// TODO: Use poison instead of undef
for (const MachineOperand &Def : MI.defs())
Copy link
Contributor

@jayfoad jayfoad Apr 8, 2025

Choose a reason for hiding this comment

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

How can this intrinsic get multiple defs in the first place?

Copy link
Contributor

@arsenm arsenm Apr 13, 2025

Choose a reason for hiding this comment

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

It can't

Copy link
Collaborator Author

@rovka rovka Apr 23, 2025

Choose a reason for hiding this comment

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

It can when we return a struct type.

B.buildUndef(Def);
MI.eraseFromParent();
return true;
}
default: {
if (const AMDGPU::ImageDimIntrinsicInfo *ImageDimIntr =
AMDGPU::getImageDimIntrinsicInfo(IntrID))
Expand Down
1 change: 0 additions & 1 deletion llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4701,7 +4701,6 @@ AMDGPURegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
case Intrinsic::amdgcn_set_inactive_chain_arg:
case Intrinsic::amdgcn_permlane64:
case Intrinsic::amdgcn_ds_bpermute_fi_b32:
case Intrinsic::amdgcn_dead:
return getDefaultMappingAllVGPR(MI);
case Intrinsic::amdgcn_cvt_pkrtz:
if (Subtarget.hasSALUFloatInsts() && isSALUMapping(MI))
Expand Down
11 changes: 11 additions & 0 deletions llvm/lib/Target/AMDGPU/SIISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6629,6 +6629,11 @@ void SITargetLowering::ReplaceNodeResults(SDNode *N,
Results.push_back(LoadVal);
return;
}
case Intrinsic::amdgcn_dead: {
Copy link
Contributor

@jayfoad jayfoad Apr 8, 2025

Choose a reason for hiding this comment

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

Why do you need to handle this both here in ReplaceNodeResults and in LowerINTRINSIC_WO_CHAIN below?

Copy link
Collaborator Author

@rovka rovka Apr 23, 2025

Choose a reason for hiding this comment

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

LowerINTRINSIC_WO_CHAIN is called when legalizing ops, and ReplaceNodeResults is called sometimes when legalizing types (i.e. if one of the returned types needs integer promotion).

for (unsigned I = 0, E = N->getNumValues(); I < E; ++I)
Results.push_back(DAG.getPOISON(N->getValueType(I)));
return;
}
}
break;
}
Expand Down Expand Up @@ -9115,6 +9120,12 @@ SDValue SITargetLowering::LowerINTRINSIC_WO_CHAIN(SDValue Op,
case Intrinsic::amdgcn_mov_dpp8:
case Intrinsic::amdgcn_update_dpp:
return lowerLaneOp(*this, Op.getNode(), DAG);
case Intrinsic::amdgcn_dead: {
SmallVector<SDValue, 8> Poisons;
for (unsigned I = 0, E = Op.getNode()->getNumValues(); I != E; ++I)
Poisons.push_back(DAG.getPOISON(Op.getNode()->getValueType(I)));
Copy link
Contributor

@arsenm arsenm Apr 30, 2025

Choose a reason for hiding this comment

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

Can make this a range loop over Op->op_values

return DAG.getMergeValues(Poisons, SDLoc(Op));
}
default:
if (const AMDGPU::ImageDimIntrinsicInfo *ImageDimIntr =
AMDGPU::getImageDimIntrinsicInfo(IntrinsicID))
Expand Down
6 changes: 0 additions & 6 deletions llvm/lib/Target/AMDGPU/SIInstructions.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4508,9 +4508,3 @@ def V_ILLEGAL : Enc32, InstSI<(outs), (ins), "v_illegal"> {
let hasSideEffects = 1;
let SubtargetPredicate = isGFX10Plus;
}

// FIXME: Would be nice if we could set the register class for the destination
// register too.
def IMP_DEF_FROM_INTRINSIC: Pat<
(i32 (int_amdgcn_dead)), (IMPLICIT_DEF)>;

32 changes: 32 additions & 0 deletions llvm/test/CodeGen/AMDGPU/legalize-amdgcn.dead.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
# RUN: llc -mtriple=amdgcn-amdpal -mcpu=gfx1200 -run-pass=legalizer %s -o - | FileCheck %s

---
name: test_struct
body: |
bb.1.entry:

; CHECK-LABEL: name: test_struct
; CHECK: [[DEF:%[0-9]+]]:_(s32) = G_IMPLICIT_DEF
; CHECK-NEXT: [[DEF1:%[0-9]+]]:_(<3 x s32>) = G_IMPLICIT_DEF
; CHECK-NEXT: [[DEF2:%[0-9]+]]:_(s64) = G_IMPLICIT_DEF
; CHECK-NEXT: [[DEF3:%[0-9]+]]:_(<2 x s16>) = G_IMPLICIT_DEF
; CHECK-NEXT: [[UV:%[0-9]+]]:_(s32), [[UV1:%[0-9]+]]:_(s32), [[UV2:%[0-9]+]]:_(s32) = G_UNMERGE_VALUES [[DEF1]](<3 x s32>)
; CHECK-NEXT: $vgpr0 = COPY [[DEF]](s32)
; CHECK-NEXT: $vgpr1 = COPY [[UV]](s32)
; CHECK-NEXT: $vgpr2 = COPY [[UV1]](s32)
; CHECK-NEXT: $vgpr3 = COPY [[UV2]](s32)
; CHECK-NEXT: $vgpr4_vgpr5 = COPY [[DEF2]](s64)
; CHECK-NEXT: $vgpr6 = COPY [[DEF3]](<2 x s16>)
; CHECK-NEXT: SI_RETURN implicit $vgpr0, implicit $vgpr1, implicit $vgpr2, implicit $vgpr3, implicit $vgpr4, implicit $vgpr5, implicit $vgpr6, implicit $vgpr7
%0:_(s32), %1:_(<3 x s32>), %2:_(s64), %3:_(<2 x s16>) = G_INTRINSIC intrinsic(@llvm.amdgcn.dead)

%4:_(s32), %5:_(s32), %6:_(s32) = G_UNMERGE_VALUES %1(<3 x s32>)
$vgpr0 = COPY %0(s32)
$vgpr1 = COPY %4(s32)
$vgpr2 = COPY %5(s32)
$vgpr3 = COPY %6(s32)
$vgpr4_vgpr5 = COPY %2(s64)
$vgpr6 = COPY %3(<2 x s16>)
SI_RETURN implicit $vgpr0, implicit $vgpr1, implicit $vgpr2, implicit $vgpr3, implicit $vgpr4, implicit $vgpr5, implicit $vgpr6, implicit $vgpr7
...
Loading