[PATCH] D66967: [Attributor] ValueSimplify Abstract Attribute

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2374
+  /// \param V Value trying to unify with SimplifiedValue
+  /// \param SimplifiedValue Current simplification result.
+  static bool checkAndUpdate(Attributor &A, const AbstractAttribute &QueryingAA,
----------------
The names below are too confusing, rename one as "existing.." or sth.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2397
+    return true;
+  }
+
----------------
Deal with `undef` as well and at some point with bitcasts and stuff.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2420
+      }
+    }
+
----------------
Add a FIXME for a typecast or add the typecast (+ test).


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2436
+      indicatePessimisticFixpoint();
+  }
+
----------------
> No need for this check, I think. ...


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2530
+               : ChangeStatus ::CHANGED;
+  }
+
----------------
`genericValueTraversal` above should deal with select and phi, correct? Do we have tests?


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

https://reviews.llvm.org/D66967