[PATCH] D22726: [DAGCombine] Match shift amount by value rather than relying on common sub-expressions.

[bryant via llvm-commits]

bryant added a comment.

I'm not sure that https://reviews.llvm.org/rL284717 fixes this:

  $ llc -march=x86-64 < test/CodeGen/X86/cmp-zext-combine.ll 
  nonzero32:                              # @nonzero32
          movl    %edi, %ecx              # <====== this is still here.
          xorl    %eax, %eax
          testq   %rcx, %rcx
          setne   %al
          retq

The original issue is still present, namely that shift amounts are compared by
DAG node instead of value (from DAGCombiner.cpp):

  // fold (srl (shl x, c), c) -> (and x, cst2)
  if (N0.getOpcode() == ISD::SHL && N0.getOperand(1) == N1 &&   // here.
      isConstantOrConstantVector(N1, /* NoOpaques */ true)) {
    SDLoc DL(N);

I don't think this is an issue with vectors, so I've updated this patch to
compare by value in the scalar case.

Test cases for have also been added for the 16- and 64-bit cases, but the former
fails:

  $ llc -march=x86-64 < test/CodeGen/X86/cmp-zext-combine.ll 
  nonzero16:                              # @nonzero16
          shll    $16, %edi               # <====== bad.
          xorl    %eax, %eax
          cmpl    $65535, %edi            # imm = 0xFFFF
          seta    %al
          retq

So I'm rather inclined to follow spatel's advice and make the change in
InstCombine instead. The patch is quite small and handles cases of all
bitwidths.


Repository:
  rL LLVM

https://reviews.llvm.org/D22726