[PATCH] D128911: Emit table lookup from TargetLowering::expandCTTZ()

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7870
+    SDValue CPIdx = DAG.getConstantPool(CA, getPointerTy(TD),
+					TD.getPrefTypeAlign(Elts[0]->getType()));
+    Align Alignment = cast<ConstantPoolSDNode>(CPIdx)->getAlign();
----------------
gsocshubham wrote:
> barannikov88 wrote:
> > You should use the alignment requirement of the array (i.e. CA), not of its element. They may differ.
> From the assembly dump of SPARC/cttz.ll, I am not sure whether to use array element alignment or array alignment?
> 
> If I use array alignment `CA`, I get below assembly as compared to `SPARC/cttz.ll` assembly if array element alignment is used. What do you think? Should I update from `CPIdx` to `CA`?
> 
> ```
> f:                                      ! @f
>         .cfi_startproc
> ! %bb.0:                                ! %entry
>         mov     %o0, %o1
>         cmp %o0, 0
>         be      .LBB0_2
>         mov     %g0, %o0
> ! %bb.1:                                ! %entry
>         sub %o0, %o1, %o0
>         and %o1, %o0, %o0
>         sethi 122669, %o1
>         or %o1, 305, %o1
>         smul %o0, %o1, %o0
>         srl %o0, 27, %o0
>         sethi %hi(.LCPI0_0), %o1
>         add %o1, %lo(.LCPI0_0), %o1
>         add %o1, %o0, %o2
>         ldub [%o2+2], %o3
>         ldub [%o2+3], %o4
>         ldub [%o1+%o0], %o0
>         ldub [%o2+1], %o1
>         sll %o3, 8, %o2
>         or %o2, %o4, %o2
>         sll %o0, 8, %o0
>         or %o0, %o1, %o0
>         sll %o0, 16, %o0
>         or %o0, %o2, %o0
> ```
I meant you should call
`TD.getPrefTypeAlign(Elts->getType())`
instead of
`TD.getPrefTypeAlign(Elts[0]->getType()`
Is the above assembly a result of such change, or did you do something different?


================
Comment at: llvm/test/CodeGen/SPARC/cttz.ll:4
+
+ at f.table = internal unnamed_addr constant [32 x i8] c"\00\01\1C\02\1D\0E\18\03\1E\16\14\0F\19\11\04\08\1F\1B\0D\17\15\13\10\07\1A\0C\12\06\0B\05\0A\09", align 1
+
----------------
Unused


================
Comment at: llvm/test/CodeGen/SPARC/cttz.ll:26
+  %0 = call i32 @llvm.cttz.i32(i32 %x, i1 true)
+  %1 = icmp eq i32 %x, 0
+  %2 = select i1 %1, i32 0, i32 %0
----------------
Why not just `ret i32 %0` ?


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

https://reviews.llvm.org/D128911