This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][MC][GFX11][NFC] Update assembler tests for VOPD instructions
ClosedPublic

Authored by dp on Sep 2 2022, 7:02 AM.

Details

Summary

We also need negative tests to check constant bus limitations and VGPR bank conflicts. These tests should be added separately.

Diff Detail

Event Timeline

dp created this revision.Sep 2 2022, 7:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 2 2022, 7:02 AM
dp requested review of this revision.Sep 2 2022, 7:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 2 2022, 7:02 AM

We also need negative tests to check constant bus limitations and VGPR bank conflicts. These tests should be added separately.

It would be good to have such tests, but the actual functionality to verify the constraints in the assembler would need to be added. We currently do not check those properties in the assembly, only during CodeGen.

Joe_Nash accepted this revision.Sep 6 2022, 7:26 AM

LGTM

This revision is now accepted and ready to land.Sep 6 2022, 7:26 AM
dp added a comment.Sep 6 2022, 7:46 AM

We also need negative tests to check constant bus limitations and VGPR bank conflicts. These tests should be added separately.

It would be good to have such tests, but the actual functionality to verify the constraints in the assembler would need to be added. We currently do not check those properties in the assembly, only during CodeGen.

Thanks, I know that VOPD validation is missing in the assembler. I'm planning to add it.

foad added a comment.Sep 6 2022, 7:53 AM
In D133205#3772066, @dp wrote:

We also need negative tests to check constant bus limitations and VGPR bank conflicts. These tests should be added separately.

It would be good to have such tests, but the actual functionality to verify the constraints in the assembler would need to be added. We currently do not check those properties in the assembly, only during CodeGen.

Thanks, I know that VOPD validation is missing in the assembler. I'm planning to add it.

Is there any way that we can use the same validation code for both the assembler and SIInstrInfo::verifyInstruction?

dp added a comment.Sep 6 2022, 8:16 AM
In D133205#3772066, @dp wrote:

We also need negative tests to check constant bus limitations and VGPR bank conflicts. These tests should be added separately.

It would be good to have such tests, but the actual functionality to verify the constraints in the assembler would need to be added. We currently do not check those properties in the assembly, only during CodeGen.

Thanks, I know that VOPD validation is missing in the assembler. I'm planning to add it.

Is there any way that we can use the same validation code for both the assembler and SIInstrInfo::verifyInstruction?

IMO, this is hardly feasible because MC layer uses a different representation of instructions and operands than Codegen does. But we may be able to reuse some snippets of SIInstrInfo::verifyInstruction code.

This revision was landed with ongoing or failed builds.Sep 7 2022, 3:43 AM
This revision was automatically updated to reflect the committed changes.