[PATCH] D32605: Recognize CTLZ builtin

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

Hi Evgeny,

I think this is an interesting concept, but I share Joerg's concern. This has to, at least, calculate the cost of CTLZ, per architecture, and compare with the cost of the loop.

For example, if the loop count is known and short, and the platform has no CTLZ, then it's quite possible that the transformation will render poorer code. It doesn't have to be a very accurate cost, but something needs to be done.

You'd also need some more testing on non-x86 platforms, to make sure the decisions are good for all of them.

cheers,
--renato



================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1191
+  // step 1: Check if the loop-back branch is in desirable form.
+  {
+    if (Value *T = matchCondition(
----------------
You don't need the extra lexical blocks.

http://llvm.org/docs/CodingStandards.html


================
Comment at: test/Transforms/LoopIdiom/ctlz.ll:1
+; RUN: opt -loop-idiom < %s -mcpu=corei7 -S | FileCheck %s
+
----------------
This won't work on builders that don't build x86. Yes, we do have those. :)

http://llvm.org/docs/TestingGuide.html#platform-specific-tests


Repository:
  rL LLVM

https://reviews.llvm.org/D32605