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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
