[PATCH] D110089: [CUDA] Implement experimental support for texture lookups.

[Justin Lebar via Phabricator via cfe-commits]

jlebar added a comment.

> Depending on which particular operation is used, the arguments vary, too.

So something like

  T __nv_tex_surf_handler(name, arg1) {
    switch (name) {
      ...
      default:
        panic();
    }
  }
  
  T __nv_tex_surf_handler(name, arg1, arg2) {
    switch(...) { ... }
  }

and so on?

> I'd need to push strcmp-based runtime dispatch down to the implementation of the texture lookups with the same operand signature.

Agree.

> That's harder to generalize, as I'd have to implement string-based dispatch for quite a few subsets of the operations -- basically for each variant of cartesian product of {dimensionality, Lod, Level, Sparse}.



> Another downside is that the string comparison code will result in functions being much larger than necessary. Probably not a big thing overall, but why add overhead that would be paid for by all users and which does not buy us anything?

If it didn't buy us anything, I'd agree.  The thing I'm concerned about is readability of this code.  Which, if we want to tie it back to users, affects our ability to catch bugs in this implementation.

> Having one trivial compiler builtin that simplifies things a lot is a better trade-off, IMO.

Ah, maybe I wasn't clear then.  I'm not actually super-concerned with the compiler builtin.  It'd be nice to get rid of it if there's a clean way to do so, but if we don't, that's ok.  Basically, the builtin is just for changing `strcmp(x, "foo")` into `builtin(x) == builtin("foo")`.  Fine.

What I'm more concerned with is the spaghetti of macros here to do something as simple as a series of overloaded functions.  It seems like a premature optimization, and I don't feel confident I can check it for bugs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110089