tra added a comment. In D88345#2298879 <https://reviews.llvm.org/D88345#2298879>, @jlebar wrote:
> OK, now I'm starting to I understand this change.. > > Before, in function scope, we allow static const/non-const `__shared__`, and > allow static const so long as it's not `__device__` or `__constant__`. > > - `static` -> error? (I understood us saying above that it is, but now that > I read the code, isn't it saying it's an error?) > - `static const` -> allowed > - `static __device__` -> error > - `static const __device__` -> error > - `static __constant__` -> error > - `static const __constant__` -> error > > After, in function scope, the rule is, allow static const/non-const > `__shared__` or anything that's `static const`. > > - `static` -> error, must be const > - `static const` -> allowed > - `static __device__` -> error, must be const > - `static const __device__` -> allowed > - `static __constant__` -> error, must be const > - `static const __constant__` -> allowed > > I guess my question when I write out this table is, why shouldn't it be like > this? > > - `static` -> allowed > - `static const` -> allowed > - `static __device__` -> allowed > - `const static __device__` -> allowed > - `static __constant__` -> error, must be const > - `const static __constant__` -> allowed It should. I did mention in a previous comment that `> Looks like the const-ness check should not be there, either`. I need to revise the patch. > This makes some sense to me because we're saying, "`__constant__` must be > const", otherwise, anything goes. Except that NVCC allows non-const `__constant__`, too. Generally speaking, C++ does not care about the attributes. While technically `__constant__` is not changeable from the device code, not specifying `const` is a missed optimization/diagnostic opportunity, but not an error per se. It does not affect how the variable is emitted, but rather what user can do with it and that's beyond the scope of this patch. I don't think it warrants a hard error. A warning, perhaps, that non-const `__constant__` is probably an error? > Or here's another way of thinking about it. You're saying that `static` and > `static __device__` in function scope are the same as a `__device__` variable > in block scope. And a `__device__` variable in block scope doesn't have to > be const (right?). So why the extra restriction on function-scope static? Something like that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88345/new/ https://reviews.llvm.org/D88345 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
