[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