[PATCH] D38862: Add must alias info to ModRefInfo.
Sanjoy Das via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 11 22:56:58 PST 2017
sanjoy requested changes to this revision.
sanjoy added a comment.
This revision now requires changes to proceed.
Unless you have a specific reason not to, I think we should merge https://reviews.llvm.org/D41091 into this one -- that'll make the patch easier to review (usually these kinds of things go the other way -- splitting the patch makes it easier to review :) ).
Meanwhile, I have some minor comments inline.
================
Comment at: include/llvm/Analysis/AliasAnalysis.h:122
+
+ /// MRI_Must is provided for completeness, but no routines will return only
+ /// MRI_Must today.
----------------
Nit s/`MRI_Must`/`ModRefInfo::Must`/ here and elsewhere
================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:824
// This operand aliases 'Object' and call reads and writes into it.
Result = ModRefInfo::ModRef;
break;
----------------
Add a comment here on why tracking `MustAlias` this way is correct despite the early exit (IIUC this is because if `Result = ModRefInfo::ModRef` the we don't use `MustAlias`)?
================
Comment at: lib/Analysis/GlobalsModRef.cpp:132
+ /// This method must align in functionality with clearMust().
+ inline ModRefInfo globalClearMust(int I) const {
+ return ModRefInfo(I & static_cast<int>(ModRefInfo::ModRef));
----------------
This isn't clearing the must bit though, right? It is clearing the `MayReadAnyGlobal` bit?
Also the `inline` here is redundant -- member functions defined within their class are implicitly `inline`.
https://reviews.llvm.org/D38862
More information about the llvm-commits
mailing list