[PATCH] D152798: [AMDGPU][ValueTracking] Handle amdgcn intrinsics that cannot create poison

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

In D152798#4417317 <https://reviews.llvm.org/D152798#4417317>, @nikic wrote:

> In D152798#4417179 <https://reviews.llvm.org/D152798#4417179>, @foad wrote:
>
>>>> I was wondering about that case myself. I don't see the benefit of saying that a load-like intrinsic can return poison, unless the compiler recognizes it as a load well enough to do things like forwarding a previously stored value through it.
>>>
>>> Not sure I follow. Do you want to define your load intrinsics are performing an implicit freeze? You could do that, but I'm not sure it is desirable.
>>
>> I don't understand the big picture here: what possible benefit is there to telling the compiler that the value that is loaded can (or can't) be poison, when the compiler knows nothing else about the provenance of that value?
>>
>> (By contrast, for a //normal// IR load, I can see that tracking the poison-ness of the loaded value might be useful, so that IR passes can add or remove store->load pairs without affecting poison-ness.)
>
> Okay, I think I get what you mean now. You want to say the load result is non-poison/undef, because the compiler will never replace the load with a literal undef/poison value due to lack of optimization support, and as such you can't introduce practical miscompiles with it. This makes me pretty uncomfortable, because you'd effectively be lying, just in a way that shouldn't matter. Maybe @nlopes has some insight on whether this could cause any specific issues.

Right. I have a similar issue with the arguments passed into GPU kernels, which are launched by hardware:

  define amdgpu_cs void @f(i32 %x, i32 %y) {
    ...
  }

The compiler will never see IR for a call to this function. You're not //allowed// to call kernels from IR. The argument values `%x` and `%y` are set up by hardware, and I think I'd like to mark them as noundef so the compiler can assume they are never poison.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152798