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

Meador Inge via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 2 08:26:54 PST 2015


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





More information about the llvm-commits mailing list