[flang] Add flags controlling whether to run the fir alias tags pass #68595
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This interface allows (HL)FIR passes to add TBAA information to fir.load and fir.store. If present, these TBAA tags take precedence over those added during CodeGen. We can't reuse mlir::LLVMIR::AliasAnalysisOpInterface because that uses the mlir::LLVMIR namespace so it tries to define methods for fir operations in the wrong namespace. But I did re-use the tbaa tag type to minimise boilerplate code. The new builders are to preserve the old interface without the tbaa tag.
For TBBAA attribute types.
See RFC at https://discourse.llvm.org/t/rfc-propagate-fir-alias-analysis-information-using-tbaa/73755 This pass adds TBAA tags to all accesses to non-pointer/target dummy arguments. These TBAA tags tell LLVM that these accesses cannot alias: allowing better dead code elimination, hoisting out of loops, and vectorization. Each function has its own TBAA tree so that accesses between funtions MayAlias after inlining. I also included code for adding tags for local allocations and for global variables. Enabling all three kinds of tag is known to produce a miscompile and so these are disabled by default. But it isn't much code and I thought it could be interesting to play with these later if one is looking at a benchmark which looks like it would benefit from more alias information. I'm open to removing this code too. TBAA tags are also added separately by TBAABuilder during CodeGen. TBAABuilder has to run during CodeGen because it adds tags to box accesses, many of which are implicit in FIR. This pass cannot (easily) run in CodeGen because fir::AliasAnalysis has difficulty tracing values between blocks, and by the time CodeGen runs, structured control flow has already been lowered. Coming in follow up patches - Change CodeGen/TBAABuilder to use TBAAForest to add tags within the same per-function trees as are used here (delayed to a later patch to make it easier to revert) - Command line argument processing to actually enable the pass
This is important to ensure that tags end up in the same trees that were created in the FIR TBAA pass. If they are in different trees then everything in one tree will be assumed to MayAlias with everything in the other tree. This leads to poor performance. @vzakhari requested that the old (not-per-function) trees are maintained so I left the old test intact.
This is fallout from getting argument names from fir.declare operations instead of fir.bindc attributes.
tblah
requested review from
banach-space,
kiranchandramohan,
Renaud-K and
vzakhari
The ultimate intention is to have this pass enabled by default whenever we are optimizing for speed. But for now, just add the arguments so this can be more easily tested. Previous PR in this series: llvm#68437
tblah
force-pushed
the
ecclescake/tbaa-arg-not-default
branch
from
ba18888 to
9168aa0
Compare
|
Does |
Not in this patch while -falias-analysis is kept separate and non-default. But that will be the intention in the future. |
banach-space
approved these changes
Oct 11, 2023
tblah
added a commit
that referenced
this pull request
Oct 12, 2023
The ultimate intention is to have this pass enabled by default whenever we are optimizing for speed. But for now, just add the arguments so this can be more easily tested. PR: #68595
|
Merged manually with ac0015f |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This does not enable the pass by default
The ultimate intention is to have this pass enabled by default whenever
we are optimizing for speed. But for now, just add the arguments so this
can be more easily tested.
Previous PR in this series: #68437
This PR is only for review of ca6751e (and any later fixups)