[PATCH] D103788: [InstCombine] Eliminate casts to optimize ctlz operation

[Datta Nagraj via Phabricator via llvm-commits]

datta.nagraj marked an inline comment as done.
datta.nagraj added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:959
+                                           A->getType()->getScalarSizeInBits());
+    return replaceInstUsesWith(
+        Trunc,
----------------
spatel wrote:
> We usually prefer to do something like:
>   Value *NarrowCtlz = Builder.CreateIntrinsic(...);
>   return BinaryOperator::CreateAdd(NarrowCtlz, WidthDiff);
> 
> The instcombine caller function then handles the replace uses and transfers the name of the existing value to the new value, so you don't have to do that explicitly. You should see a cosmetic (but not functional) difference if you regenerate the CHECK lines in the test files with that change.
Done. Actually, I had refered the visitAdd function to see how to match intrinsic patterns, and there they were using the replaceInstUsesWith, but the approach you suggested looks clean. Made the changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103788