[PATCH] D65280: Add a pass to lower is.constant intrinsics Needs ReviewPublic

[Joerg Sonnenberger via Phabricator via llvm-commits]

joerg marked 3 inline comments as done.
joerg added inline comments.


================
Comment at: include/llvm/Analysis/InstructionSimplify.h:258
+                           OptimizationRemarkEmitter *ORE = nullptr,
+                           bool SimplifyCFG = false);
 
----------------
echristo wrote:
> Really would like to avoid boolean arguments like this.
Right, this is the one big part of the change I am absolutely not sure about. I was considering passing a pointer to a vector down to replaceAndRecursivelySimplify. The contract would be to return the work list of all modified uses. The caller can then iterate the list and see if any branch instructions are affected. 


================
Comment at: lib/Analysis/InstructionSimplify.cpp:5047
 
+static Value *simplifyBranch(BranchInst *BI, bool SimplifyCFG) {
+  if (!SimplifyCFG || BI->isUnconditional())
----------------
echristo wrote:
> This seems to return nullptr everywhere?
It does, trying to preserve the style of the other functions here. The question might become irrelevant when going with the work list idea.


================
Comment at: lib/Transforms/Scalar/LowerIsConstantIntrinsic.cpp:51
+  bool NewChange, Changed = false;
+
+  do {
----------------
arsenm wrote:
> Should early exit if there are is no is_constant declaration, or just visit uses of it rather than search the whole function for a rarely used intrinsic?
As mentioned on IRC, llvm.is.constant is a family of intrinsics depending on the type of the argument. There doesn't seem to be an easy way to check if a given template-like intrinsic is used.


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

https://reviews.llvm.org/D65280