azabaznov added a comment.
Herald added a subscriber: Naghasan.
> How about we clarify with Khronos whether it would be sufficient to add a
> restriction like:
>
>> Program scope blocks are only supported when program scope variables feature
>> is supported.
That's sounds good to me. Especially because this states nothing about if block
has captures/no captures. Originally, I was thinking about something like:
//blocks in constant address space require program scope variables feature
support//, but that's too much relies on concrete ABI.
================
Comment at: clang/lib/AST/ExprConstant.cpp:9554
+
+ const auto &OpenCLFeaturesMap =
+ Info.Ctx.getTargetInfo().getSupportedOpenCLOpts();
----------------
Anastasia wrote:
> What test case covers this change? It feels like something we should try to
> diagnose earlier...
The test case which exactly was added. Since blocks in constant address space
are disallowed at this point, we can treat all other blocks with no captures
not as constant expressions - it will make CodeGen generate block literal in
local scope for blocks with no captures. See `buildGlobalBlock `and
`CodeGenModule::GetAddrOfGlobalBlock` for details.
================
Comment at: clang/lib/CodeGen/CGOpenCLRuntime.cpp:191
+// feature support.
+bool CGOpenCLRuntime::blockCanBeGlobal() {
+ const auto &LO = CGM.getLangOpts();
----------------
Anastasia wrote:
> This function feels like something that belongs better to `OpenCLOptions`
> rather than `CodeGen`?
IIRC we can't query Sema from a CodeGen... The alternative would be to
introduce a new language option, which is not desirable.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:8024
if (T->isBlockPointerType()) {
+ if ((T.getAddressSpace() == LangAS::opencl_constant) &&
+ !getOpenCLOptions().areProgramScopeVariablesSupported(
----------------
Anastasia wrote:
> Good spot, I didn't feel the intent was to allow qualifying the block by an
> address space... but I don't think this was ever clarified. I feel that the
> assumption was that blocks would always be in global address space...
Well, unfortunately there are already few cases in CTS coverage which rely on
that... Note that this is an address space of a block, but not a block literal.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112230/new/
https://reviews.llvm.org/D112230
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
