[PATCH] D63382: [InstCombine] fold a shifted zext to a select

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

IMO, this is the right canonicalization for IR because it's the smallest form. Also if the code was already in this form, it might have profile data on the select condition that would benefit codegen (for example, we might want to compare and branch on this instead of turning it into bit-logic). I've made several related changes to prefer 'select' in IR over bithacks.

The only problem that I see is that the backend (DAGCombiner) isn't prepared to generically reverse this pattern yet (x86 might have some code that can be lifted):

  define i32 @shl_zext_bool(i1 %t) {
    %ext = zext i1 %t to i32
    %shl = shl i32 %ext, 7
    ret i32 %shl
  }
  
  define i32 @sel_zext_bool(i1 %t) {
    %shl= select i1 %t, i32 128, i32 0
    ret i32 %shl
  }



  $ ./llc -o - selsh.ll -mtriple=aarch64--
  shl_zext_bool:                          // @shl_zext_bool
  	and	w8, w0, #0x1
  	lsl	w0, w8, #7
  	ret
  
  sel_zext_bool:                          // @sel_zext_bool
  	tst	w0, #0x1
  	mov	w8, #128
  	csel	w0, w8, wzr, ne
  	ret



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp:668-669
   Constant *C1;
   if (match(Op1, m_Constant(C1))) {
     Constant *C2;
     Value *X;
----------------
zvi wrote:
> lebedev.ri wrote:
> > I think you want to add the fold here, no reason to restrict it to identical vectors
> What is the policy for handling non-splat cases, should they always be handled when possible or are they limited only to certain cases to reduce compile-time?
There's no official policy AFAIK. If it's easy enough to apply the transform to an arbitrary vector constant, then I'd prefer we do it. 

There should not be much compile-time overhead because (1) either way, we're checking each element of the vector constant to determine if it's a splat and (2) there probably aren't that many non-splat vector constants to begin with.


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

https://reviews.llvm.org/D63382