Page MenuHomePhabricator

[AMDGPU] Make V_SAT_PK_U8_I16 a True16 Instruction
ClosedPublic

Authored by Joe_Nash on Oct 7 2022, 12:06 PM.

Details

Summary

The return type is two u8 packed into a 16 bit VGPR, so this instruction
should be True16.

Diff Detail

Event Timeline

Joe_Nash created this revision.Oct 7 2022, 12:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2022, 12:06 PM
Joe_Nash requested review of this revision.Oct 7 2022, 12:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2022, 12:06 PM

It's not clear to me if this instruction was ever selected. I don't see any ISel pattern or reference in tablegen or c++. In the synthesized tablgen defs, I see list<dag> Pattern = [(set i32:$vdst, (null_frag (i32 (VOP3Mods0 i32:$src0))))]; which I think doesn't get used because of the null_frag? Anyway, if it is being selected, do I need to update the pattern for the type change?

dp accepted this revision.Oct 7 2022, 2:03 PM

MC changes LGTM.

This revision is now accepted and ready to land.Oct 7 2022, 2:03 PM
arsenm added a comment.Oct 7 2022, 4:37 PM

It's not clear to me if this instruction was ever selected. I don't see any ISel pattern or reference in tablegen or c++. In the synthesized tablgen defs, I see list<dag> Pattern = [(set i32:$vdst, (null_frag (i32 (VOP3Mods0 i32:$src0))))]; which I think doesn't get used because of the null_frag? Anyway, if it is being selected, do I need to update the pattern for the type change?

If it's not getting selected, it probably should be. I believe this corresponds to llvm.sadd.sat

It's not clear to me if this instruction was ever selected. I don't see any ISel pattern or reference in tablegen or c++. In the synthesized tablgen defs, I see list<dag> Pattern = [(set i32:$vdst, (null_frag (i32 (VOP3Mods0 i32:$src0))))]; which I think doesn't get used because of the null_frag? Anyway, if it is being selected, do I need to update the pattern for the type change?

If it's not getting selected, it probably should be. I believe this corresponds to llvm.sadd.sat

Thanks. I made an issue for this https://github.com/llvm/llvm-project/issues/58266

This revision was automatically updated to reflect the committed changes.