This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Use module flag to get code object version at IR level
ClosedPublic

Authored by cfang on Feb 1 2023, 11:20 PM.

Details

Summary

This patch introduces a mechanism to check the code object version from the module flag, This avoids checking from command line.
In case the module flag is missing, we use the current default code object version supported in the compiler.

For tools whose inputs are not IR, we may need other approach (directive, for example) to check the code
object version, That will be in a separate patch later.

For LIT tests update, we directly add module flag if there is only a single code object version associated with all checks in one file.
In cause of multiple code object version in one file, we use the "sed" method to "clone" the checks to achieve the goal.

Diff Detail

Event Timeline

cfang created this revision.Feb 1 2023, 11:20 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2023, 11:20 PM
cfang requested review of this revision.Feb 1 2023, 11:20 PM
Herald added a reviewer: sstefan1. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
arsenm added inline comments.Feb 2 2023, 3:04 AM
llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
351

Should not unreachable on unknown version. Either should error or just try to handle as the latest version. We have a few other unreachable that need fixing too.

Should add a test with 0/1 and a large number

llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp
414

Cann move this into AMDGPUInformationCache

llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
897

Only need to call this once (Can we cache it in the streamer or read from the an argument?)

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp
547

Don't need parens

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
172

As a separate patch need to stop unreachable on unhandled version

193–203

Should also clean up all these argument offset queries to return a single struct or something

cfang added inline comments.Feb 2 2023, 11:47 AM
llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp
414

Should we introduce a private field of Module *M in AMDGPUInformationCache?
InformationCache itself does not have this field.
Otherwise, we will have to pass a Module parameter in the call to check code object version each time.

arsenm added inline comments.Feb 2 2023, 1:22 PM
llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp
414

You can directly parse the code object version into the info cache, it won't be shared between modules

cfang marked 3 inline comments as done.Feb 2 2023, 3:55 PM
cfang added inline comments.
llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
351

Plan to work on with the unreachable code object version as another patch.
Add a test case as suggested.

llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp
414

Keep code object version in AMDGPUInformationCache when the object is created.

llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
897

Pass as an argument.

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
172

Plan to handle out of range code object version in another patch.

193–203

Have to think over what to do. Better to be in another patch.

cfang updated this revision to Diff 494458.Feb 2 2023, 4:01 PM
cfang marked an inline comment as done.

Update based on Matt's comments:

  1. Add a test case for out of range code object version;
  2. Move code object version into AMDGPUInformationCache
  3. Pass code object version as an argument in MetadataStreamerMsgPackV3::getHSAKernelProps

4 remove unnecessary parens

The unreachable code object version will be handled in a later patch, together with other considerations.

arsenm added inline comments.Feb 2 2023, 5:10 PM
llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
351

it's still a dead break, better to just not make this unreachable in the first place

llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp
141

can do it in initializer list

171

const

232

const

llvm/test/CodeGen/AMDGPU/unsupported-code-object-version.ll
7

you can't rely on this message, at least use report_fatal_error

cfang updated this revision to Diff 494486.Feb 2 2023, 6:00 PM

Update based on Matt's comments.

Use report_fatal_error in stead of llvm_unreachable to report un-supported code object version.

arsenm accepted this revision.Feb 2 2023, 6:01 PM
This revision is now accepted and ready to land.Feb 2 2023, 6:01 PM