This is an archive of the discontinued LLVM Phabricator instance.

[mlir][AMDGPU] Add --chipset option to AMDGPUToROCDL
ClosedPublic

Authored by krzysz00 on Jul 6 2022, 2:29 PM.

Details

Summary

Because the buffer descriptor structure (the V#) has no backwards-compatibility
guarentees, and since said guarantees have been violated in practice
(see https://github.com/llvm/llvm-project/issues/56323 ), and since
the targetIsRDNA attribute isn't something that higher-level clients can set
in general, make the lowering of the amdgpu dialect to rocdl take a --chipset
option.

Note that this option is a string because adding a parser for the Chipset
struct to llvm::cl wasn't working out.

Diff Detail

Event Timeline

krzysz00 created this revision.Jul 6 2022, 2:29 PM
Herald added a project: Restricted Project. · View Herald Transcript
krzysz00 requested review of this revision.Jul 6 2022, 2:29 PM
kosarev added a reviewer: Restricted Project.Jul 7 2022, 3:09 AM
herhut accepted this revision.Jul 7 2022, 3:13 AM
herhut added inline comments.
mlir/include/mlir/Conversion/AMDGPUToROCDL/AMDGPUToROCDL.h
11

nit: include ordering

mlir/lib/Conversion/AMDGPUToROCDL/AMDGPUToROCDL.cpp
33

nit: should this be private?

This revision is now accepted and ready to land.Jul 7 2022, 3:13 AM
krzysz00 added inline comments.Jul 7 2022, 7:57 AM
mlir/lib/Conversion/AMDGPUToROCDL/AMDGPUToROCDL.cpp
33

I'm not sure I see the argument for it. It's a read-only field on rewrite pattern, and if people feel inclined to mess with it between rewrites that's on them

This revision was automatically updated to reflect the committed changes.