Details
- Reviewers
nikic - Commits
- rG3e992d81afc3: [InferAlignment] Enable InferAlignment pass by default
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/test/Transforms/PhaseOrdering/X86/excessive-unrolling.ll | ||
---|---|---|
135–136 | any idea why this regressed? |
llvm/test/Transforms/PhaseOrdering/X86/excessive-unrolling.ll | ||
---|---|---|
135–136 | Loop unrolling causes generation of instructions after InferAlignment runs, so this is not caught by it. |
llvm/lib/Passes/PassBuilderPipelines.cpp | ||
---|---|---|
1281 | can this also be replaced with the new pass? |
llvm/lib/Passes/PassBuilderPipelines.cpp | ||
---|---|---|
1281 | I do not believe so, because this uses SCEV information whereas the InferAlignment pass does not. It can probably be implemented in a later patch though. |
LGTM -- based on stats this should be low impact, so let's try it. This very runs the pass two times (before and after unrolling), which is probably more than we need longer term but for now should hopefully avoid regressions.
I'm confused...
This pass regresses alignment information in a lot of tests. I've also observed in when compiling real programs.
What's the reason for enabling this?
The test regressions in this diff are just an artifact of inference moving into a different pass (the tests run instcombine as part of the pipeline, while is no longer responsible for alignment inference). If you see end-to-end regressions in alignment information, that would be a problem though. Do you have an example?
Ah, I see.
No, didn't see end-to-end regressions. I was looking somewhere in the middle of the pipeline only.
But this may impact alias analysis, depending on how often and there this new pass runs. Instcombine runs more often than this pass. We need to monitor for regressions.
We've started seeing ubsan unaligned access errors after this commit. I'm not yet sure the code is correct, so just a heads up for now.
can this also be replaced with the new pass?