This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][MC][GFX11] Correct v_dot2_f16_f16 and v_dot2_bf16_bf16
ClosedPublic

Authored by dp on Aug 2 2022, 7:03 AM.

Details

Summary

Enable SGPRs for the following operands of these opcodes:

  • src operands of VOP3 variant.
  • src2 operand of DPP variants.

See this bug for more information.

Diff Detail

Event Timeline

dp created this revision.Aug 2 2022, 7:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 2 2022, 7:03 AM
dp requested review of this revision.Aug 2 2022, 7:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 2 2022, 7:03 AM
arsenm accepted this revision.Aug 2 2022, 7:11 AM
This revision is now accepted and ready to land.Aug 2 2022, 7:11 AM
Joe_Nash added a comment.EditedAug 2 2022, 8:11 AM

This looks fine for v_dot2_f16_f16.
For v_dot2_bf16_bf16 I am a bit unsure. Because we treat bf16 as int16, will ISel work correctly if we use the FP16InputMods? We don't have a codegen test using modifiers for v_dot2_bf16_bf16, one should probably be added to llvm.amdgcn.fdot2.bf16.bf16.ll . IntOpSelMods is a potential alternative SrcModifiers operand class to use.

foad added a subscriber: rampitec.Aug 2 2022, 8:19 AM

For v_dot2_bf16_bf16 I am a bit unsure. Because we treat bf16 as int16, will ISel work correctly if we use the FP16InputMods? We don't have a codegen test using modifiers for v_dot2_bf16_bf16, one should probably be added to llvm.amdgcn.fdot2.bf16.bf16.ll . IntOpSelMods is a potential alternative SrcModifiers operand class to use.

+ @rampitec. This already came up on D129084.

rampitec accepted this revision.Aug 2 2022, 11:24 AM

For v_dot2_bf16_bf16 I am a bit unsure. Because we treat bf16 as int16, will ISel work correctly if we use the FP16InputMods? We don't have a codegen test using modifiers for v_dot2_bf16_bf16, one should probably be added to llvm.amdgcn.fdot2.bf16.bf16.ll . IntOpSelMods is a potential alternative SrcModifiers operand class to use.

+ @rampitec. This already came up on D129084.

At least from the encoding point of view modifiers are present, same as with f16. From the codegen point of view we cannot use these anyway because bf16 is not a legal type.

This revision was landed with ongoing or failed builds.Aug 3 2022, 5:11 AM
This revision was automatically updated to reflect the committed changes.