[PATCH] D40749: Modify ModRefInfo values using static inline method abstractions [NFC].

Sanjoy Das via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 4 17:38:05 PST 2017


sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

lgtm with some comments inline



================
Comment at: include/llvm/Analysis/AliasAnalysis.h:232
+// Wrapper method strips bits significant only in FunctionModRefBehavior,
+// to obtain a valid ModRefInfo. UB if those bits are not stripped out.
+// The benefit of using the wrapper is that if ModRefInfo enum changes,
----------------
> UB if those bits are not stripped out.

I don't see how this is relevant here.


================
Comment at: lib/Analysis/AliasAnalysis.cpp:267
+        ModRefInfo ArgMask;
+        if (isModSet(ArgModRefCS2))
           ArgMask = MRI_ModRef;
----------------
Shouldn't this be `isMod`?  Before it was checking that the return value of `getArgModRefInfo` was `== MRI_Mod`.


================
Comment at: lib/Analysis/AliasAnalysis.cpp:360
+  if (Loc.Ptr) {
+    AliasResult AR = alias(MemoryLocation::get(L), Loc);
+    if (AR == NoAlias)
----------------
This change seems unnecessary.


================
Comment at: lib/Analysis/AliasAnalysis.cpp:377
   if (Loc.Ptr) {
+    AliasResult AR = alias(MemoryLocation::get(S), Loc);
     // If the store address cannot alias the pointer in question, then the
----------------
This change seems unnecessary.


================
Comment at: lib/Analysis/AliasAnalysis.cpp:404
   if (Loc.Ptr) {
+    AliasResult AR = alias(MemoryLocation::get(V), Loc);
     // If the va_arg address cannot alias the pointer in question, then the
----------------
This change seems unnecessary.


================
Comment at: lib/Analysis/AliasAnalysisEvaluator.cpp:35
 static cl::opt<bool> PrintRef("print-ref", cl::ReallyHidden);
+static cl::opt<bool> PrintMod("print-mod", cl::ReallyHidden);
 static cl::opt<bool> PrintModRef("print-modref", cl::ReallyHidden);
----------------
This change seems unnecessary.


================
Comment at: lib/Analysis/AliasAnalysisEvaluator.cpp:258
+        PrintModRefResults("Just Mod", PrintMod, I, Pointer, F.getParent());
+        ++ModCount;
+        break;
----------------
This change seems unnecessary.


================
Comment at: lib/Analysis/AliasAnalysisEvaluator.cpp:283
         break;
+      case MRI_Mod:
+        PrintModRefResults("Just Mod", PrintMod, *C, *D, F.getParent());
----------------
This change seems unnecessary.


================
Comment at: lib/Analysis/AliasAnalysisEvaluator.cpp:328
   // Display the summary for mod/ref analysis
-  int64_t ModRefSum = NoModRefCount + ModCount + RefCount + ModRefCount;
+  int64_t ModRefSum = NoModRefCount + RefCount + ModCount + ModRefCount;
   if (ModRefSum == 0) {
----------------
This change seems unnecessary.


https://reviews.llvm.org/D40749





More information about the llvm-commits mailing list