[PATCH] D158603: [AMDGPU][TargetMachine] Handle case when +extended-image-insts is set, and the user forces +wave64
Juan Manuel Martinez CaamaƱo via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 24 03:43:25 PDT 2023
jmmartinez added a comment.
In D158603#4609902 <https://reviews.llvm.org/D158603#4609902>, @arsenm wrote:
> First, last I knew comgr was not using clang/-mlink-builtin-bitcode (although maybe this was fixed?).
> [...]
> @uses_nothing isn't even getting the +wavefrontsize64, which is doubly broken. I believe this was working correctly at one point, so did this regress? clang is broken here, so that should be fixed first. If comgr is still not correctly using -mlink-builtin-bitcode, it should implement an approximately equivalent hack to append the features if there are still blockers to doing so.
Thanks for pointing this out. Comgr uses the `Linker::linkInModule` function directly. I wasn't aware that there was a difference, but I see now that using `-mlink-builtin-bitcode` does `Gen->CGM().addDefaultFunctionDefinitionAttributes(F);` and does internalization for every function in the builtin bitcode. `Linker::linkInModule` only keeps the attributes in the definition coming in from the builtin bitcode and ignores those in the declaration.
However, there is a TODO in that function...
void CodeGenModule::addDefaultFunctionDefinitionAttributes(llvm::Function &F) {
llvm::AttrBuilder FuncAttrs(F.getContext());
getDefaultFunctionAttributes(F.getName(), F.hasOptNone(),
/* AttrOnCallSite = */ false, FuncAttrs);
// TODO: call GetCPUAndFeaturesAttributes?
F.addFnAttrs(FuncAttrs);
}
If I set up a call to `GetCPUAndFeaturesAttributes(GlobalDecl(), F)` any target-features present in the definition are overriden (so it overrides the +extended-image-insts).
---
I have one question though. Why device_libs has that target specific attribute? Shouldn't it be deduced later in the optimization pipeline since the function has calls to image intrinsics?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158603/new/
https://reviews.llvm.org/D158603
More information about the llvm-commits
mailing list