[PATCH] D29316: Add predicateinfo intrinsic, analysis pass, and basic NewGVN support

[Daniel Berlin via Phabricator via llvm-commits]

dberlin marked 8 inline comments as done.
dberlin added inline comments.


================
Comment at: include/llvm/Transforms/Utils/PredicateInfo.h:244
+  struct Result {
+    Result(std::unique_ptr<PredicateInfo> &&PredInfo)
+        : PredInfo(std::move(PredInfo)) {}
----------------
Prazek wrote:
> take unique_ptr by value. This ensures that the pointer value sinks (and it is also less characters to read, and it is easier for the optimizer to optimize it) 
This does not actually work, afaict.
If you try it, it does not compile no matter what because unique ptrs can't be copied by value, only moved.

If you want me to do something here, exact code appreciated.

This is also a direct copy of the idiom we are using in other analysis passes :)



================
Comment at: lib/Transforms/Scalar/NewGVN.cpp:1103
+                                                       const BasicBlock *B) {
+  CmpInst *CI = dyn_cast<CmpInst>(I);
+  // See if our operands are equal to those of a previous predicate, and if so,
----------------
Prazek wrote:
> auto *
As the summary says, this part of code is mainly here so folks can play with it.
I plan on submitting the newgvn changes separately from the rest, and will clean them up prior to submission.


https://reviews.llvm.org/D29316