rsmith added a comment.
We need to figure out what should happen in the OpenCL case, but the rest seems
fine.
================
Comment at: lib/Sema/SemaChecking.cpp:3360
}
- if (AtomTy.isConstQualified() ||
+ if ((Form != Load && Form != LoadCopy && AtomTy.isConstQualified()) ||
AtomTy.getAddressSpace() == LangAS::opencl_constant) {
----------------
The `LoadCopy` check is redundant; only the GNU `__atomic_load` builtin has the
`LoadCopy` form. But see below, we can avoid this duplicated condition with
some refactoring.
================
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)
----------------
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.)
================
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();
}
}
----------------
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?
Repository:
rC Clang
https://reviews.llvm.org/D47618
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
