[PATCH] D14234: [ARM] Recognize bit-reversal idioms

[James Molloy via llvm-commits]

Hi Meador,

Thanks for having a look. Ouch, I'll fix that crash pronto, thanks!

> I haven't worked through all the logic yet, but this seems like a very
specific optimization.  How often have you found that it fires in
practice?  Any benchmark results?

I'd counter that it is the obvious optimization to select that instruction.
At the moment there's no way to get RBIT selected apart from by using an
intrinsic. This at least makes the obvious bit-reversal code in C do the
right thing. I've found it firing at least a couple of times in the
test-suite, and very much so in a third-party benchmark suite that I can't
share. In that suite I get about a 200% speedup in one test with this
optimization.

I'm happy to change the capitalization - it was mainly force of habit.

Cheers,

James

On Mon, 2 Nov 2015 at 16:27 Meador Inge via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> meadori added a subscriber: meadori.
> meadori added a comment.
>
> I haven't worked through all the logic yet, but this seems like a very
> specific optimization.  How often have you found that it fires in
> practice?  Any benchmark results?
>
> Also, changing the test case to:
>
>   --- test.ll   2015-11-02 10:21:04.000000000 -0600
>   +++ crash.ll  2015-11-02 10:20:51.000000000 -0600
>   @@ -11,7 +11,7 @@
>    entry:
>      %a.lobit = lshr i32 %a, 31
>      %and1 = lshr i32 %a, 29
>   -  %0 = and i32 %and1, 2
>   +  %0 = and i32 %and1, 1
>      %and6 = lshr i32 %a, 27
>      %1 = and i32 %and6, 4
>      %and11 = lshr i32 %a, 25
>
> causes the following assert to fire:
>
>   Assertion failed: (bitPosition < getBitWidth() && "Bit position out of
> bounds!")
>
>
> ================
> Comment at: lib/Target/ARM/ARMISelLowering.cpp:8896
> @@ +8895,3 @@
> +// Try and detect an RBIT - bit reversal.
> +static SDValue performRBITCombine(SDNode *N,
> +                                  TargetLowering::DAGCombinerInfo &DCI,
> ----------------
> Any particular reason this is `performRBITCombine` instead of
> `PerformRBITCombine`?  The coding standard calls for functions to start
> with lowercase letters, but it looks like most of the functions in this
> file are already start with uppercase letters.
>
>
> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D14234
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151102/e25e6cf8/attachment.html>