jfb added a subscriber: yaxunl.
jfb added inline comments.
================
Comment at: lib/Sema/SemaChecking.cpp:3361
+ if ((Form != Load && Form != LoadCopy && AtomTy.isConstQualified()) ||
AtomTy.getAddressSpace() == LangAS::opencl_constant) {
Diag(DRE->getLocStart(), diag::err_atomic_op_needs_non_const_atomic)
----------------
rsmith wrote:
> We also need to figure out what to do about this -- should an atomic load
> from a constant address space be valid? (It seems a little pointless to use
> an *atomic* load here, but not obviously wrong.)
I think it's mostly harmless except when the address space is actually constant
(not C++ constant) because in these cases a wide atomic might need cmpxchg to
perform a read, and that will fail writing. Maybe @Anastasia can chime in for
OpenCL?
================
Comment at: lib/Sema/SemaChecking.cpp:3368-3374
} else if (Form != Load && Form != LoadCopy) {
if (ValType.isConstQualified()) {
Diag(DRE->getLocStart(), diag::err_atomic_op_needs_non_const_pointer)
<< Ptr->getType() << Ptr->getSourceRange();
return ExprError();
}
}
----------------
rsmith wrote:
> It would be a little nicer to change this `else if` to a plain `if` and
> conditionalize the diagnostic instead.
>
> Can you track down whoever added the address space check to the C11 atomic
> path and ask them if they really meant for it to not apply to the GNU atomic
> builtins?
It was @yaxunl in https://reviews.llvm.org/D28691
Nobody asked about GNU atomic builtins with OpenCL in that review. I'll let
@yaxunl chime in.
Repository:
rC Clang
https://reviews.llvm.org/D47618
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
