[PATCH] D90382: [InstCombine] foldSelectRotate - generalize to foldSelectFunnelShift

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D90382#2375931 <https://reviews.llvm.org/D90382#2375931>, @nlopes wrote:

> In D90382#2375905 <https://reviews.llvm.org/D90382#2375905>, @nikic wrote:
>
>> @nlopes I think we should adjust the funnel shift definition to say that it blocks poison on one operand if the shift amount is zero. Basically the poison semantics should be "as if" the funnel shift were expanded, which does include an explicit select for the zero shift amount case.
>
> The issue with having instructions block poison is that then you can't remove those instruction. Right now only `select` and `freeze` block poison. So I would try to stay away of making funnel shift block poison.

That only applies to basic instructions. Here we're talking about a function call. The poison semantics of the call depend on the implementation of the called function. If we implemented `llvm.fshr.i8` in LLVM IR, it would look something like...

  define i8 @llvm.fshr.i8(i8 %x, i8 %y, i8 %shamt) {
    %xyz = ...
    %ret = select i1 %cmp, i8 %y, i8 %xyz
    ret i8 %ret
  }

... and would have corresponding poison semantics. Now, because it is an intrinsic, we have the technical capability of giving it different semantics, but I don't see why we should go out of the way to specify something non-standard here.

Note that we currently don't do any special modelling of poison wrt calls (apart from noundef attributes), so we are currently not making use of the fact that for most intrinsics "poison in poison out" holds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90382