rjmccall added inline comments.
================
Comment at: clang/lib/CodeGen/CGStmt.cpp:2260
Result.addFnAttr(llvm::Attribute::ReadNone);
- else if (ReadOnly)
+ Result.addFnAttr(llvm::Attribute::NoReadThreadID);
+ } else if (ReadOnly)
----------------
Hmm, my comment here got lost somehow.
This looks like a new semantic assumption. Can we split this patch so that
this is separate from the introduction of the new attribute? I know we were
assuming this until a few weeks ago, but still, it's generally best practice to
do representation changes independent from semantic changes.
Also, this is basically saying that relying on the thread identity is a
side-effect that needs to be reflected in the constraints. Is there
documentation justifying that assumption? Doesn't this make some kinds of
existing code invalid?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132352/new/
https://reviews.llvm.org/D132352
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
