[PATCH] D141464: [X86]: Match (xor TSize - 1, ctlz) to `bsr` instead of `lzcnt` + `xor`

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Target/X86/X86InstrCompiler.td:2254
+// emitting extra xor + lzcnt.
+def : Pat<(xor (ctlz GR16:$src), 15), (BSR16rr GR16:$src)>;
+def : Pat<(xor (ctlz GR32:$src), 31), (BSR32rr GR32:$src)>;
----------------
craig.topper wrote:
> This doesn't produce the correct value for $src being 0. The ctlz would return 16 and the xor would turn that to 31. BSR16rr will produce an undefined value.
> 
> I think this is only valid for the zero_undef nodes.
> This doesn't produce the correct value for $src being 0. The ctlz would return 16 and the xor would turn that to 31. BSR16rr will produce an undefined value.
> 
> I think this is only valid for the zero_undef nodes.

I think you're right although `call i32 @llvm.ctlz.i32(i32, i1 true)` doesn't seem to match `ctlz_zero_undef`.
Also taking a look at `lzcnt` def:
```
  def LZCNT16rr : I<0xBD, MRMSrcReg, (outs GR16:$dst), (ins GR16:$src),
                    "lzcnt{w}\t{$src, $dst|$dst, $src}",
                    [(set GR16:$dst, (ctlz GR16:$src)), (implicit EFLAGS)]>,
                    XS, OpSize16, Sched<[WriteLZCNT]>;
  def LZCNT16rm : I<0xBD, MRMSrcMem, (outs GR16:$dst), (ins i16mem:$src),
                    "lzcnt{w}\t{$src, $dst|$dst, $src}",
                    [(set GR16:$dst, (ctlz (loadi16 addr:$src))),
                     (implicit EFLAGS)]>, XS, OpSize16, Sched<[WriteLZCNTLd]>;

  def LZCNT32rr : I<0xBD, MRMSrcReg, (outs GR32:$dst), (ins GR32:$src),
                    "lzcnt{l}\t{$src, $dst|$dst, $src}",
                    [(set GR32:$dst, (ctlz GR32:$src)), (implicit EFLAGS)]>,
                    XS, OpSize32, Sched<[WriteLZCNT]>;
...
```

Which is just set as `ctlz` so I thought it was just a very confusing overloaded wording in the TD. Do you know what I'm missing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141464