-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[AMDGPU][NFC] Add an optional DSE pass during CodeGenPrepare #172069
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-backend-amdgpu Author: Gang Chen (cmc-rep) ChangesFull diff: https://github.com/llvm/llvm-project/pull/172069.diff 1 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
index 8a831f7915882..b8b6d2427d7d0 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
@@ -104,6 +104,7 @@
#include "llvm/Transforms/IPO/GlobalDCE.h"
#include "llvm/Transforms/IPO/Internalize.h"
#include "llvm/Transforms/Scalar.h"
+#include "llvm/Transforms/Scalar/DeadStoreElimination.h"
#include "llvm/Transforms/Scalar/EarlyCSE.h"
#include "llvm/Transforms/Scalar/FlattenCFG.h"
#include "llvm/Transforms/Scalar/GVN.h"
@@ -1413,6 +1414,9 @@ void AMDGPUPassConfig::addCodeGenPrepare() {
addPass(createAMDGPULowerKernelArgumentsPass());
TargetPassConfig::addCodeGenPrepare();
+ // TODO: Remove DSE when LoadStoreVectorizer is enhanced to handle
+ // partially overlapping vector-stores.
+ addPass(createDeadStoreEliminationPass());
if (isPassEnabled(EnableLoadStoreVectorizer))
addPass(createLoadStoreVectorizerPass());
@@ -2183,6 +2187,9 @@ void AMDGPUCodeGenPassBuilder::addCodeGenPrepare(AddIRPass &addPass) const {
addPass(AMDGPULowerKernelArgumentsPass(TM));
Base::addCodeGenPrepare(addPass);
+ // TODO: Remove DSE when LoadStoreVectorizer is enhanced to handle
+ // partially overlapping vector-stores.
+ addPass(DSEPass());
if (isPassEnabled(EnableLoadStoreVectorizer))
addPass(LoadStoreVectorizerPass());
|
arsenm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should have already happened in the middle end optimizer. This would also need a phase ordering test
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
I have made this extra DSE pass optional, and it is off by default. We need this downstream since we have added a major IR transform in the middle end optimizer that require extra DSE afterwards. I could add this downstream only. However, I feel that people may find it a useful option in some other cases. |
By default, this option is off for now.
|
This is not NFC |
This sounds like it could use better placement. This must have a phase ordering test that shows its useful, so keep it with this transform |
|
Will add this downstream where I can come up a relevant test. |
By default, the option is off for now.