Anastasia added a comment. In D89520#2335628 <https://reviews.llvm.org/D89520#2335628>, @rsmith wrote:
> In D89520#2334477 <https://reviews.llvm.org/D89520#2334477>, @Anastasia wrote: > >>> I couldn't find any spec justification for accepting the kinds of cases >>> that D20090 <https://reviews.llvm.org/D20090> accepts, so a reference to >>> where in the OpenCL specification >>> this is permitted would be useful. >> >> Thanks for fixing this. I agree that the original change was not compliant >> with the spec. OpenCL indeed doesn't allow constant folding for array >> bounds. The idea of the change was to allow using expressions that are >> compile time constant in the array bound because this doesn't result in VLA. >> >> Regarding the spec reference, I think we can refer to the section 6.5.3 >> describing variables in the `__constant` address space: >> >> These variables are required to be initialized and the values >> used to initialize these variables must be a compile time constant. >> Writing to such a variable results in a compile-time error. >> >> I.e. the `__constant` address space variables are semantically similar to >> `constexpr` in C++. > > I don't see any way to read this wording that way. That talks about how the > variables are initialized and says they're immutable, but I can't find > anything in the OpenCL specification that says they can be used in integer > constant expressions. The C definition of "integral constant expression" does > not allow the use of variables: > > C11 6.6/6: "An integer constant expression shall have integer type and shall > only have operands that are integer constants, enumeration constants, > character constants, sizeof expressions whose results are integer constants, > _Alignof expressions, and floating constants that are the immediate operands > of casts." > > ... and absent a modification to that, nor would OpenCL. That said, I'm happy > to take your word for it that the OpenCL specification //intends// for such > variable uses to be permitted in constant expressions. Yes, I agree it is not explicit. I will attempt to improve it. However considering that your change doesn't add this feature but modifies existing implementation, I think it makes sense to go ahead. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89520/new/ https://reviews.llvm.org/D89520 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
