-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Add a pass to collect dropped var stats for MIR #120780
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
This patch uses the DroppedVariableStats class to add dropped variable statistics for MIR passes. Reland 1c082c9
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.
You should've moved DroppedVariableStats stuff out of llvm/Passes in advance.
At a glance, they don't depend on LLVMPass any more. (#120711)
#define LLVM_CODEGEN_DROPPEDVARIABLESTATSMIR_H | ||
|
||
#include "llvm/CodeGen/MachineFunction.h" | ||
#include "llvm/Passes/DroppedVariableStats.h" |
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.
It's odd to depend on LLVMPasses here.
This reverts commit 3bf91ad. (llvmorg-20-init-16123-g3bf91ad2a9c7) `llvm/CodeGen` should not depend on `llvm/Passes`.
I have reverted this. I could fix them though. |
I have already commented on this three times... #115566 (comment) ...and my feedback still hasn't been addressed. If you reapply this again without actually addressing the problems with this patch, I will request your commit access to be withdrawn. |
@nikic I am very sorry about not addressing the patch issues, I will make sure to not include so much code into MachineFunctionPass.h. I had applied some aggressive filtering in my email and didn't see those comments, but I understand it is not an excuse. Do you have some recommendations on how to redesign this better to make it so that the build times are smaller? |
@chapuni The reason I designed it this way, is because if I move all the With this error:
This is because libclang/codegen doesn't link against libllvm/codegen however, it does link against libllvm/passes. If I cannot have libllvm/codegen not link against passes, then I am unsure of where I can put the code so that I can calculate dropped statistics for both IR and MIR. Do you have an idea of what could be done? |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/1904 Here is the relevant piece of the build log for the reference
|
|
This patch attempts to reland #120780 while addressing the issues that caused the patch to be reverted. Namely: 1. The patch had included code from the llvm/Passes directory in the llvm/CodeGen directory. 2. The patch increased the backend compile time by 2% due to adding a very expensive include in MachineFunctionPass.h The patch has been re-structured so that there is no dependency between the llvm/Passes and llvm/CodeGen directory, by moving the base class, `class DroppedVariableStats` to the llvm/IR directory. The expensive include in MachineFunctionPass.h has been changed to contain forward declarations instead of other header includes which was pulling a ton of code into MachineFunctionPass.h and should resolve any issues when it comes to compile time increase.
…686) This patch attempts to reland llvm/llvm-project#120780 while addressing the issues that caused the patch to be reverted. Namely: 1. The patch had included code from the llvm/Passes directory in the llvm/CodeGen directory. 2. The patch increased the backend compile time by 2% due to adding a very expensive include in MachineFunctionPass.h The patch has been re-structured so that there is no dependency between the llvm/Passes and llvm/CodeGen directory, by moving the base class, `class DroppedVariableStats` to the llvm/IR directory. The expensive include in MachineFunctionPass.h has been changed to contain forward declarations instead of other header includes which was pulling a ton of code into MachineFunctionPass.h and should resolve any issues when it comes to compile time increase.
This patch attempts to reland llvm#120780 while addressing the issues that caused the patch to be reverted. Namely: 1. The patch had included code from the llvm/Passes directory in the llvm/CodeGen directory. 2. The patch increased the backend compile time by 2% due to adding a very expensive include in MachineFunctionPass.h The patch has been re-structured so that there is no dependency between the llvm/Passes and llvm/CodeGen directory, by moving the base class, `class DroppedVariableStats` to the llvm/IR directory. The expensive include in MachineFunctionPass.h has been changed to contain forward declarations instead of other header includes which was pulling a ton of code into MachineFunctionPass.h and should resolve any issues when it comes to compile time increase. (cherry picked from commit 92f916f)
This patch attempts to reland llvm#120780 while addressing the issues that caused the patch to be reverted. Namely: 1. The patch had included code from the llvm/Passes directory in the llvm/CodeGen directory. 2. The patch increased the backend compile time by 2% due to adding a very expensive include in MachineFunctionPass.h The patch has been re-structured so that there is no dependency between the llvm/Passes and llvm/CodeGen directory, by moving the base class, `class DroppedVariableStats` to the llvm/IR directory. The expensive include in MachineFunctionPass.h has been changed to contain forward declarations instead of other header includes which was pulling a ton of code into MachineFunctionPass.h and should resolve any issues when it comes to compile time increase.
This patch attempts to reland llvm#120780 while addressing the issues that caused the patch to be reverted. Namely: 1. The patch had included code from the llvm/Passes directory in the llvm/CodeGen directory. 2. The patch increased the backend compile time by 2% due to adding a very expensive include in MachineFunctionPass.h The patch has been re-structured so that there is no dependency between the llvm/Passes and llvm/CodeGen directory, by moving the base class, `class DroppedVariableStats` to the llvm/IR directory. The expensive include in MachineFunctionPass.h has been changed to contain forward declarations instead of other header includes which was pulling a ton of code into MachineFunctionPass.h and should resolve any issues when it comes to compile time increase.
This patch attempts to reland llvm#120780 while addressing the issues that caused the patch to be reverted. Namely: 1. The patch had included code from the llvm/Passes directory in the llvm/CodeGen directory. 2. The patch increased the backend compile time by 2% due to adding a very expensive include in MachineFunctionPass.h The patch has been re-structured so that there is no dependency between the llvm/Passes and llvm/CodeGen directory, by moving the base class, `class DroppedVariableStats` to the llvm/IR directory. The expensive include in MachineFunctionPass.h has been changed to contain forward declarations instead of other header includes which was pulling a ton of code into MachineFunctionPass.h and should resolve any issues when it comes to compile time increase.
This patch uses the DroppedVariableStats class to add dropped variable statistics for MIR passes.
Reland 1c082c9