[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp:67 + if (auto *GA = dyn_cast(Op.getGlobal())) +return cast(GA->getOperand(0)); return cast(Op.getGlobal()); arsenm wrote: > gandhi21299 wrote: > >

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

arsenm added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp:67 + if (auto *GA = dyn_cast(Op.getGlobal())) +return cast(GA->getOperand(0)); return cast(Op.getGlobal()); gandhi21299 wrote: > arsenm wrote: > > I thought

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp:67 + if (auto *GA = dyn_cast(Op.getGlobal())) +return cast(GA->getOperand(0)); return cast(Op.getGlobal()); arsenm wrote: > I thought aliases could

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

arsenm added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp:67 + if (auto *GA = dyn_cast(Op.getGlobal())) +return cast(GA->getOperand(0)); return cast(Op.getGlobal()); I thought aliases could include embedded bitcasts

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

This revision was automatically updated to reflect the committed changes. Closed by commit rG0567f0333176: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols (authored by gandhi21299). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added inline comments. Comment at: clang/test/CodeGenCUDA/amdgpu-alias-undef-symbols.cu:3 + +// RUN: %clang -target x86_64-unknown-linux-gnu --offload-arch=gfx906 --cuda-device-only -nogpulib -nogpuinc -x hip -emit-llvm -S -o - %s \ +// RUN: -fgpu-rdc -O3 -mllvm

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

MaskRay added inline comments. Comment at: clang/test/CodeGenCUDA/amdgpu-alias-undef-symbols.cu:3 + +// RUN: %clang -target x86_64-unknown-linux-gnu --offload-arch=gfx906 --cuda-device-only -nogpulib -nogpuinc -x hip -emit-llvm -S -o - %s \ +// RUN: -fgpu-rdc -O3 -mllvm

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added a comment. @thakis can you please check if this solution is sufficient? Thanks for bringing it up Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 updated this revision to Diff 380110. gandhi21299 added a comment. added -target option in the test amdgpu-alias-undef-symbols.cu Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707 Files:

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

yaxunl added a comment. @gandhi21299 you may need to add "-target x86_64-unknown-linux-gnu" to your codegen test to avoid issue with Darwin toolchain. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

thakis added a comment. This breaks tests on Mac: http://45.33.8.238/mac/37119/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

This revision was automatically updated to reflect the committed changes. Closed by commit rG03375a3fb33b: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols (authored by gandhi21299). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

yaxunl accepted this revision. yaxunl added a comment. This revision is now accepted and ready to land. LGTM. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added a comment. Passed ePSDB Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 updated this revision to Diff 379530. gandhi21299 added a comment. add a restrictions to what architecture AlwaysInliner should run on, updated the inline-calls.ll test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added inline comments. Comment at: llvm/test/CodeGen/AMDGPU/inline-calls.ll:3 ; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs < %s | FileCheck %s -; RUN: llc -march=r600 -mcpu=redwood -verify-machineinstrs < %s | FileCheck %s @tstellar

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added a comment. Passed internal CI Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 updated this revision to Diff 378218. gandhi21299 added a comment. added -nogpulib and -nogpuinc flags to amdgpu-alias-undef-symbols.cu Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707 Files:

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 updated this revision to Diff 377922. gandhi21299 added a comment. refreshing patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707 Files: clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 updated this revision to Diff 376559. gandhi21299 added a comment. - Since callees may alias to a function pointer, it makes sense for `getCalleeFunction(...)` to return a `Function` which is a cast of the operand of a `GlobalAlias`. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 updated this revision to Diff 376564. gandhi21299 added a comment. - eliminated changes in SIISelLowering Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707 Files:

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added a comment. inline-calls.ll failed on gfx908 due to the change in SIISelLowering.cpp, line 3015. Without the change, there is a failure in AMDGPUResourceAnalysis.cpp, line 65 because Op.getGlobal() is not a Function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 reclaimed this revision. gandhi21299 added a comment. Sorry, that was a mistake. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707 ___ cfe-commits

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp:96-97 if (Function* F = dyn_cast(A.getAliasee())) { + if (A.getLinkage() != GlobalValue::InternalLinkage) +continue; A.replaceAllUsesWith(F);

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

yaxunl added inline comments. Comment at: llvm/test/CodeGen/AMDGPU/inline-calls.ll:1 ; RUN: llc -march=amdgcn -mcpu=tahiti -verify-machineinstrs < %s | FileCheck %s need to add check for gfx906 and gfx1030 Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

yaxunl added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp:96-97 if (Function* F = dyn_cast(A.getAliasee())) { + if (A.getLinkage() != GlobalValue::InternalLinkage) +continue; A.replaceAllUsesWith(F); If

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 updated this revision to Diff 375655. gandhi21299 added a comment. - declare failure when lowering an accessor of a callee which is not a function, in GlobalISel Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added a comment. @yaxunl Should inline-calls.ll be converted into an expected failing test or removed? (to avoid cast failure in AMDGPUResourceAnalysis to break the test) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 updated this revision to Diff 375284. gandhi21299 marked an inline comment as done. gandhi21299 added a comment. - added the `REQUIRES` line as requested by Sam Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added a comment. In D109707#3016438 , @gandhi21299 wrote: > - replaced a `cast` with a `dyn_cast` since the return value from > `getCalleeFunction()` is not always a Function > - `RUN on line 2` was causing 2 more scalar registers to be

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

yaxunl added a comment. Pls update the description of the patch and also make sure it passes internal CI. Comment at: clang/test/CodeGenCUDA/amdgpu-alias-undef-symbols.cu:1 +// RUN: %clang --offload-arch=gfx906 --cuda-device-only -x hip -emit-llvm -S -o - %s \ +// RUN:

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added a comment. It does not look like function calls are supported yet in AMDGPUCallLowering, is that correct? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 updated this revision to Diff 375079. gandhi21299 added a comment. - Declare an unhandled call lowering in SelectionDAG when a callee is encountered which cannot be casted into a Function - I am still investigating the effects on GlobalISel side of things, there seems to be a

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

arsenm added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp:65 - return cast(Op.getGlobal()); + return dyn_cast(Op.getGlobal()); } gandhi21299 wrote: > arsenm wrote: > > I think this is not the right place for this. If

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp:65 - return cast(Op.getGlobal()); + return dyn_cast(Op.getGlobal()); } arsenm wrote: > I think this is not the right place for this. If we can determine

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

arsenm added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp:65 - return cast(Op.getGlobal()); + return dyn_cast(Op.getGlobal()); } I think this is not the right place for this. If we can determine the callee function,

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 updated this revision to Diff 374369. gandhi21299 added a comment. - refreshing patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707 Files: clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 updated this revision to Diff 374354. gandhi21299 added a comment. - replaced a `cast` with a `dyn_cast` since the return value from `getCalleeFunction()` is not always a Function - `RUN on line 2` was causing 2 more scalar registers to be used on tonga due to @func_alias not being

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 updated this revision to Diff 373257. gandhi21299 marked an inline comment as done. gandhi21299 added a comment. - Prevent removing alias if the GlobalAlias does not have internal linkage Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

arsenm added a comment. In D109707#3004869 , @gandhi21299 wrote: > Internal linkage detection works great for our purposes but it causes a > failure in llvm/test/CodeGen/AMDGPU/inline-calls.ll due to `@func_alias` > unable to be casted into a

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added a comment. @yaxunl I think we have two ways to go from here: 1. If appropriate, reset the maximum number of scalar registers allowed in `@kernel3` (inline-calls.ll) to fix the test. 2. Determine a stronger condition for inlining. Repository: rG LLVM Github Monorepo

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added a comment. Internal linkage detection works great for our purposes but it causes a failure in llvm/test/CodeGen/AMDGPU/inline-calls.ll due to `@func_alias` unable to be casted into a `Function`. If we pass through that, the `@kernel3` causes the error: `scalar registers (98)

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

yaxunl added a comment. In D109707#3004108 , @gandhi21299 wrote: > @yaxunl Under what criteria should an alias not be removed? If the linkage of the alias is not internal, then it should not be removed. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added a comment. @yaxunl Under what criteria should an alias not be removed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707 ___ cfe-commits

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

yaxunl added a comment. I think you may try fixing the following line: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp#L97 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 updated this revision to Diff 372573. gandhi21299 added a comment. - converted the HIP test into a CUDA test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707 Files:

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 updated this revision to Diff 372551. gandhi21299 added a comment. - added the include header for HIP runtime Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707 Files:

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5069 // where aliases aren't supported. - if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX() && !RawTriple.isAMDGPU()) CmdArgs.push_back("-mconstructor-aliases");

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 updated this revision to Diff 372528. gandhi21299 marked an inline comment as done. gandhi21299 added a comment. - set `GlobalOpt` parameter to false by default to disallow alias elimination when the options EarlyInlineAll and EnableFunctionCalls are true and false, respectively.

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 marked an inline comment as done. gandhi21299 added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5069 // where aliases aren't supported. - if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX() && !RawTriple.isAMDGPU())

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

yaxunl added a comment. In D109707#2998095 , @aeubanks wrote: > should the `amdgpu-early-inline-all` flag be deleted? No. It is still used by HIP. We will deprecate it in the feature, but it is not ready yet. Repository: rG LLVM Github Monorepo

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

aeubanks added a comment. should the `amdgpu-early-inline-all` flag be deleted? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707 ___ cfe-commits mailing list

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

yaxunl added a comment. We cannot disable early inline all since this will cause performance regressions. Instead, the early inline all pass should be fixed so it does not remove aliases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

arsenm added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5069 // where aliases aren't supported. - if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX() && !RawTriple.isAMDGPU()) CmdArgs.push_back("-mconstructor-aliases"); This

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

arsenm added a comment. While I think the early inliner is largely obsolete, it should still handle aliases correctly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109707/new/ https://reviews.llvm.org/D109707

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

gandhi21299 created this revision. gandhi21299 added reviewers: yaxunl, aeubanks. Herald added subscribers: foad, kerbowa, hiraditya, tpr, nhaehnle, jvesely, arsenm. gandhi21299 requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits,