[AMDGPU] Move AMDGPUAttributorPass to full LTO post link stage
#102086
Conversation
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
@llvm/pr-subscribers-backend-amdgpu Author: Shilei Tian (shiltian) ChangesCurrently Full diff: https://github.com/llvm/llvm-project/pull/102086.diff 1 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
index c8fb68d1c0b0c..812f88ed67583 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
@@ -301,6 +301,11 @@ static cl::opt<bool> EnableSIModeRegisterPass(
cl::init(true),
cl::Hidden);
+static cl::opt<bool> EnableAMDGPUAttributorInFullLTO(
+ "amdgpu-attributor-enable-full-lto",
+ cl::desc("Enable AMDGPUAttributorPass in full LTO post link stage"),
+ cl::init(false), cl::Hidden);
+
// Enable GFX11.5+ s_singleuse_vdst insertion
static cl::opt<bool>
EnableInsertSingleUseVDST("amdgpu-enable-single-use-vdst",
@@ -749,6 +754,8 @@ void AMDGPUTargetMachine::registerPassBuilderCallbacks(PassBuilder &PB) {
// module is partitioned for codegen.
if (EnableLowerModuleLDS)
PM.addPass(AMDGPULowerModuleLDSPass(*this));
+ if (EnableAMDGPUAttributorInFullLTO)
+ PM.addPass(AMDGPUAttributorPass(*this));
});
PB.registerRegClassFilterParsingCallback(
|
|
If we want to run it by default, there is probably no point to run it in default pipeline. cc @jdoerfert |
|
We need to run the pass once after linking in all (supported) pipelines. Pipelines (for now): Pipelines (hopefully soon, https://github.com/jdoerfert/llvm-project/tree/thin_lto): I think we need to move it late, as I said (and did) in the closed world patch after we moved it. Otherwise we miss out on closed-world and optimizations of runtimes and cross TU. |
Do we want to move the pass directly to full lto pipeline? It will not run in full lto post link stage right now. Is it by design? |
|
ping |
3c4dc8d to
ec12f76
Compare
AMDGPUAttributorPass in full LTOAMDGPUAttributorPass to full LTO post link stage
04478b8 to
22c2b26
Compare
bc5beb7 to
1c39d4c
Compare
| @@ -734,21 +739,15 @@ void AMDGPUTargetMachine::registerPassBuilderCallbacks(PassBuilder &PB) { | |||
| PM.addPass(createCGSCCToFunctionPassAdaptor(std::move(FPM))); | |||
| }); | |||
|
|
|||
| // FIXME: Why is AMDGPUAttributor not in CGSCC? | |||
| PB.registerOptimizerLastEPCallback( | |||
| [this](ModulePassManager &MPM, OptimizationLevel Level) { | |||
There was a problem hiding this comment.
Does this mean we never run amdgpuattributor without LTO?
There was a problem hiding this comment.
That is correct. For HIP (w/ or w/o rdc) and OpenMP, we always run LTO. Need to check OpenCL. If not, we probably want to run it as well.
There was a problem hiding this comment.
That is an issue. We still should be running this in non-LTO
There was a problem hiding this comment.
Then we will run it twice in LTO.
ee47897 to
659844f
Compare
Currently `AMDGPUAttributorPass` is registered in default optimizer pipeline. This will allow the pass to run in default pipeline as well as at thinLTO post link stage. However, it will not run in full LTO post link stage. This patch adds an option to run it in full LTO. It is set to false by default.
659844f to
bcb7768
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/3392 Here is the relevant piece of the build log for the reference: |
|
This and my previously reverted PR, as well as a SimplifyCFG related PR (not from me but requested to be reverted yesterday), almost broke the blender test in the same way. This can't be coincidental IMHO. It must expose something wrong in the AMDGPU backend. |
|
Blender assumes -fno-gpu-rdc, which does not use LTO. It uses the default optimization pipeline. Maybe the failure is due to removing the pass from the default optimization pipeline? |
I thought we do even for no-gpu-rdc? This is from running blender test on my side: |
For fno-gpu-rdc case, the input to this lld invocation is a relocatable object file. No LTO is performed. |
|
A light breeze can cause blender to fail. Any random code perturbation will break it until the WWM spilling patches are committed |
Agree. But for this PR I think it may actually caught a real issue |
Very likely the case. I checked the IR of my previously reverted PR. The only difference between the working version and mine is an extra address space cast, which is legit. Then it fails the blender test. |
Then the question would be, is AMDGPUAttributorPass mandatory? |
It may be. Did you try this PR with internal PSDB? |
It cannot be mandatory. Something is broken |
Blender is part of PSDB for amd-staging. Even if we remove it from buildbot, chances are PSDB will fail and what is worse is that we have to do manual regression analysis to identify the causing commit. |
Does it mean I can't merge anything if blender test fails, even not because of my changes? I have a list of planned PRs that will be based on those reverted PRs. |
I suspect this PR may cause regressions for other HIP apps in PSDB. If that happens, it has to be reverted. I recommend opening a gerrit review to trigger an internal PSDB. We cannot break PSDB. |
Oh not this one. I have reverted this one. I was referring to #100952. |
* 'main' of https://github.com/llvm/llvm-project: (700 commits) [SandboxIR][NFC] SingleLLVMInstructionImpl class (llvm#102687) [ThinLTO]Clean up 'import-assume-unique-local' flag. (llvm#102424) [nsan] Make #include more conventional [SandboxIR][NFC] Use Tracker.emplaceIfTracking() [libc] Moved range_reduction_double ifdef statement (llvm#102659) [libc] Fix CFP long double and add tests (llvm#102660) [TargetLowering] Handle vector types in expandFixedPointMul (llvm#102635) [compiler-rt][NFC] Replace environment variable with %t (llvm#102197) [UnitTests] Convert a test to use opaque pointers (llvm#102668) [CodeGen][NFCI] Don't re-implement parts of ASTContext::getIntWidth (llvm#101765) [SandboxIR] Clean up tracking code with the help of emplaceIfTracking() (llvm#102406) [mlir][bazel] remove extra blanks in mlir-tblgen test [NVPTX][NFC] Update tests to use bfloat type (llvm#101493) [mlir] Add support for parsing nested PassPipelineOptions (llvm#101118) [mlir][bazel] add missing td dependency in mlir-tblgen test [flang][cuda] Fix lib dependency [libc] Clean up remaining use of *_WIDTH macros in printf (llvm#102679) [flang][cuda] Convert cuf.alloc for box to fir.alloca in device context (llvm#102662) [SandboxIR] Implement the InsertElementInst class (llvm#102404) [libc] Fix use of cpp::numeric_limits<...>::digits (llvm#102674) [mlir][ODS] Verify type constraints in Types and Attributes (llvm#102326) [LTO] enable `ObjCARCContractPass` only on optimized build (llvm#101114) [mlir][ODS] Consistent `cppType` / `cppClassName` usage (llvm#102657) [lldb] Move definition of SBSaveCoreOptions dtor out of header (llvm#102539) [libc] Use cpp::numeric_limits in preference to C23 <limits.h> macros (llvm#102665) [clang] Implement -fptrauth-auth-traps. (llvm#102417) [LLVM][rtsan] rtsan transform to preserve CFGAnalyses (llvm#102651) Revert "[AMDGPU] Move `AMDGPUAttributorPass` to full LTO post link stage (llvm#102086)" [RISCV][GISel] Add missing tests for G_CTLZ/CTTZ instruction selection. NFC Return available function types for BindingDecls. (llvm#102196) [clang] Wire -fptrauth-returns to "ptrauth-returns" fn attribute. (llvm#102416) [RISCV] Remove riscv-experimental-rv64-legal-i32. (llvm#102509) [RISCV] Move PseudoVSET(I)VLI expansion to use PseudoInstExpansion. (llvm#102496) [NVPTX] support switch statement with brx.idx (reland) (llvm#102550) [libc][newhdrgen]sorted function names in yaml (llvm#102544) [GlobalIsel] Combine G_ADD and G_SUB with constants (llvm#97771) Suppress spurious warnings due to R_RISCV_SET_ULEB128 [scudo] Separated committed and decommitted entries. (llvm#101409) [MIPS] Fix missing ANDI optimization (llvm#97689) [Clang] Add env var for nvptx-arch/amdgpu-arch timeout (llvm#102521) [asan] Switch allocator to dynamic base address (llvm#98511) [AMDGPU] Move `AMDGPUAttributorPass` to full LTO post link stage (llvm#102086) [libc][math][c23] Add fadd{l,f128} C23 math functions (llvm#102531) [mlir][bazel] revert bazel rule change for DLTITransformOps [msan] Support vst{2,3,4}_lane instructions (llvm#101215) Revert "[MLIR][DLTI][Transform] Introduce transform.dlti.query (llvm#101561)" [X86] pr57673.ll - generate MIR test checks [mlir][vector][test] Split tests from vector-transfer-flatten.mlir (llvm#102584) [mlir][bazel] add bazel rule for DLTITransformOps OpenMPOpt: Remove dead include [IR] Add method to GlobalVariable to change type of initializer. (llvm#102553) [flang][cuda] Force default allocator in device code (llvm#102238) [llvm] Construct SmallVector<SDValue> with ArrayRef (NFC) (llvm#102578) [MLIR][DLTI][Transform] Introduce transform.dlti.query (llvm#101561) [AMDGPU][AsmParser][NFC] Remove a misleading comment. (llvm#102604) [Arm][AArch64][Clang] Respect function's branch protection attributes. (llvm#101978) [mlir] Verifier: steal bit to track seen instead of set. (llvm#102626) [Clang] Fix Handling of Init Capture with Parameter Packs in LambdaScopeForCallOperatorInstantiationRAII (llvm#100766) [X86] Convert truncsat clamping patterns to use SDPatternMatch. NFC. [gn] Give two scripts argparse.RawDescriptionHelpFormatter [bazel] Add missing dep for the SPIRVToLLVM target [Clang] Simplify specifying passes via -Xoffload-linker (llvm#102483) [bazel] Port for d45de80 [SelectionDAG] Use unaligned store/load to move AVX registers onto stack for `insertelement` (llvm#82130) [Clang][OMPX] Add the code generation for multi-dim `num_teams` (llvm#101407) [ARM] Regenerate big-endian-vmov.ll. NFC [AMDGPU][AsmParser][NFCI] All NamedIntOperands to be of the i32 type. (llvm#102616) [libc][math][c23] Add totalorderl function. (llvm#102564) [mlir][spirv] Support `memref` in `convert-to-spirv` pass (llvm#102534) [MLIR][GPU-LLVM] Convert `gpu.func` to `llvm.func` (llvm#101664) Fix a unit test input file (llvm#102567) [llvm-readobj][COFF] Dump hybrid objects for ARM64X files. (llvm#102245) AMDGPU/NewPM: Port SIFixSGPRCopies to new pass manager (llvm#102614) [MemoryBuiltins] Simplify getCalledFunction() helper (NFC) [AArch64] Add invalid 1 x vscale costs for reductions and reduction-operations. (llvm#102105) [MemoryBuiltins] Handle allocator attributes on call-site LSV/test/AArch64: add missing lit.local.cfg; fix build (llvm#102607) Revert "Enable logf128 constant folding for hosts with 128bit floats (llvm#96287)" [RISCV] Add Syntacore SCR5 RV32/64 processors definition (llvm#102285) [InstCombine] Remove unnecessary RUN line from test (NFC) [flang][OpenMP] Handle multiple ranges in `num_teams` clause (llvm#102535) [mlir][vector] Add tests for scalable vectors in one-shot-bufferize.mlir (llvm#102361) [mlir][vector] Disable `vector.matrix_multiply` for scalable vectors (llvm#102573) [clang] Implement CWG2627 Bit-fields and narrowing conversions (llvm#78112) [NFC] Use references to avoid copying (llvm#99863) Revert "[mlir][ArmSME] Pattern to swap shape_cast(tranpose) with transpose(shape_cast) (llvm#100731)" (llvm#102457) [IRBuilder] Generate nuw GEPs for struct member accesses (llvm#99538) [bazel] Port for 9b06e25 [CodeGen][NewPM] Improve start/stop pass error message CodeGenPassBuilder (llvm#102591) [AArch64] Implement TRBMPAM_EL1 system register (llvm#102485) [InstCombine] Fixing wrong select folding in vectors with undef elements (llvm#102244) [AArch64] Sink operands to fmuladd. (llvm#102297) LSV: document hang reported in llvm#37865 (llvm#102479) Enable logf128 constant folding for hosts with 128bit floats (llvm#96287) [RISCV][clang] Remove bfloat base type in non-zvfbfmin vcreate (llvm#102146) [RISCV][clang] Add missing `zvfbfmin` to `vget_v` intrinsic (llvm#102149) [mlir][vector] Add mask elimination transform (llvm#99314) [Clang][Interp] Fix display of syntactically-invalid note for member function calls (llvm#102170) [bazel] Port for 3fffa6d [DebugInfo][RemoveDIs] Use iterator-inserters in clang (llvm#102006) ... Signed-off-by: Edwiin Kusuma Jaya <kutemeikito0905@gmail.com>
This is basically same as #102086 but reverts some test case changes that are no longer needed.
This is basically same as llvm#102086 but reverts some test case changes that are no longer needed. (cherry picked from commit 862f504) Change-Id: Icc620b3afc6eacc6fea00d91176be054feaf3a6c
Currently
AMDGPUAttributorPassis registered in default optimizer pipeline.This will allow the pass to run in default pipeline as well as at thinLTO post
link stage. However, it will not run in full LTO post link stage. This patch
moves it to full LTO.