[PATCH] D85668: [Attributor][NFC] Connect AAPotentialValues with AAValueSimplify

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 10 19:07:39 PDT 2020


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

LGTM, some minor nits to consider.



================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:4484
+        A.getAAFor<AAType>(*this, getIRPosition(), /* TrackDependence */ true,
+                           DepClassTy::OPTIONAL);
 
----------------
Nit: Even better if we add a dependence only if we use the result, in either case, OPTIONAL is the right kind :)


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:4498
+      }
     }
+  }
----------------
You don't need an else after return, that should bring all the indention to a single level here.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:4504
+      if (!askSimplifiedValueFor<AAPotentialValues>(A))
+        return false;
     return true;
----------------
early exits:
```
if (ask...)
  return true;
if (ask...)
  return true;
return false;
```



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85668/new/

https://reviews.llvm.org/D85668



More information about the llvm-commits mailing list