[PATCH] D109671: [AMDGPU] Add BFE pattern matches for constrained shifts.

[Abinav Puthan Purayil via Phabricator via llvm-commits]

abinavpp added a comment.

In D109671#2997988 <https://reviews.llvm.org/D109671#2997988>, @arsenm wrote:

> In D109671#2996887 <https://reviews.llvm.org/D109671#2996887>, @foad wrote:
>
>> This will give the wrong result when $width is zero.
>
> I'm not seeing the problem with 0?

The introduced (srl (shl $src, (and (sub 0, $width), 31)), (and (sub 0, $width), 31))
on $width = 0 evaluates to (srl (shl $src, 0), 0) = $src != (v_bfe_u32 $src, 0, 0)

The original (srl (shl $src, (sub 32, $width)), (sub 32, $width)) on $width = 0
will evaluate to (srl (shl $src, 32), 32). Assuming the ISD::SHL's semantics is
the same as that of LLVM-IR's shl, (shl $src, 32) yields a poison value. If I
understand correctly, selecting v_bfe_u32 here is acceptable but not for the
pattern introduced in this revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109671