[PATCH] D77777: [nvptx] Add `nvvm.texsurf.handle` internalizer.

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D77777#1974988 <https://reviews.llvm.org/D77777#1974988>, @tra wrote:

> In D77777#1974849 <https://reviews.llvm.org/D77777#1974849>, @hliao wrote:
>
> >
>
>
>
>
> >> NVVM IR spec is for nvidia's own compiler. It's based on LLVM, but it does not impose direct constraints on LLVM's design choices.
> > 
> > It would be an advantage and, sometimes, desirable to generate IR compatible to NVVM IR spec.
>
> I'm not against it, but I think it's OK to make different choices if we have good reasons for that. NVIDIA didn't update LLVM since they've contributed the original implementation, so by now we're both far behind the current state of NVVM and quite a bit sideways due to the things LLVM has added to NVPTX backend.
>
> >> This sounds like it may have been done that way in an attempt to work around a problem with intrinsics' constraints. We may want to check if there's a better way to do it now.
> >>  Right now both intrinsics are marked with `[IntrNoMem]` which may be the reason for compiler feeling free to move it around. We may need to give compiler correct information and then we may not need this just-in-time intrinsic replacement hack. I think it should be at least `IntrArgMemOnly` or, maybe  `IntrInaccessibleMemOrArgMemOnly`.
> > 
> > That may not exactly model the behavior as, for binding texture/surface support, in fact, it's true that there's no memory operation at all. Even with InstArgMemOnly or similar attributes, it still won't be preventable for optimizations to sink common code. Such trick is played in lots of intrinsics, such as `read.register` and etc.
>
> Can you give me an example where/how optimizer would break things? Is that because were using metadata as an argument?
>
> I've re-read NVVM docs and I can't say that I understand how it's supposed to work.
>  `metadata holding the texture or surface variable` alone is a rather odd notion and I'm not surprised that it's not handled well. In the end we do end up with a 'handle' which is an in-memory object. Perhaps it should be represented as a real variable with a metadata attribute. Then we can lower it as a handle,  can enforce that only texture/surface instructions are allowed to use it and will have a way to tell LLVM what it's allowed to do.
>
> I don't have a good picture of how it all will fit together in the end (or whether what I suggest makes sense), but the current implementation appears to be in need of rethinking.


the 1st argument in `llvm.nvvm.texsurf.hande.internal` or the 2nd one in `llvm.nvvm.texsurf.handle` must be kept as an immediate or constant value, i.e. that global variable. However, optimizations will find common code in the following

  if (cond) {
    %hnd = texsurf.handle.internal(@tex1);
  } else {
    %hnd = texsurf.handle.internal(@tex2)
  }
  = use(%hnd)

and hoist or sink it into

  if (cond) {
    %ptr = @tex1;
  } else {
    %ptr = @tex2;
  }
  %hnd = texsurf.handle.intenal(%ptr);
  = use(%hnd)

The backend cannot handle non immediate operand in `texsurf.handle`. The similar thing happens to `read.register` as well as it also assumes its argument is always an immediate value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77777