[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