Skip to content

[AMDGPU][True16][CodeGen] update VGPRimm for t16 #131021

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 1 commit into from
Mar 20, 2025
Merged

[AMDGPU][True16][CodeGen] update VGPRimm for t16 #131021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions llvm/lib/Target/AMDGPU/SIInstrInfo.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,11 @@ def cond_as_i32imm: SDNodeXForm<cond, [{
}]>;

// Copied from the AArch64 backend:
def bitcast_fpimm_to_i16 : SDNodeXForm<fpimm, [{
return CurDAG->getTargetConstant(
N->getValueAPF().bitcastToAPInt().getZExtValue(), SDLoc(N), MVT::i16);
}]>;

def bitcast_fpimm_to_i32 : SDNodeXForm<fpimm, [{
return CurDAG->getTargetConstant(
N->getValueAPF().bitcastToAPInt().getZExtValue(), SDLoc(N), MVT::i32);
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/AMDGPU/SIInstructions.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2283,7 +2283,7 @@ let True16Predicate = UseRealTrue16Insts in {
foreach vt = [f16, bf16] in {
def : GCNPat <
(VGPRImm<(vt fpimm)>:$imm),
(V_MOV_B16_t16_e64 0, $imm, 0)
(V_MOV_B16_t16_e64 0, (vt (bitcast_fpimm_to_i16 $imm)), 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

The type of immediate nodes in the selected dag doesn't really matter, they're all turning into sign extended int64_t in the end anyway. We could change all of these to just use i64 in the first place

Copy link
Contributor Author

@broxigarchen broxigarchen Mar 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Double checked in downstream branch. I think you are right.

Still merge this patch as it makes the pattern more consisitent

>;
}
}
Expand Down
Loading