User Details
- User Since
- Oct 1 2018, 7:44 AM (225 w, 6 d)
Jun 20 2022
May 24 2022
This patch relates to my previous comment: https://reviews.llvm.org/D124621#3485799
This patch relates to my previous comment: https://reviews.llvm.org/D124621#3485799
May 5 2022
May 3 2022
May 2 2022
For reference, the modified code was introduced with https://reviews.llvm.org/D80015.
Apr 29 2022
Update commit message and add FIXMEs
One thing I'm not sure about and couldn't easily find in the doc is how to reference in the commit message the bug (https://llvm.org/PR48534) this patch fixes. Is it good as is?
Apr 28 2022
Remove LLVM_DUMP_METHOD from definition
Ran wrong arc command...
Apr 27 2022
I'd appreciate reviews for this small patch. Thanks.
I'd appreciate reviews for this small patch. Thanks.
Sep 18 2021
May 14 2021
LGTM, thanks for the update.
May 7 2021
May 6 2021
This change removes the requirement on pragma when atomic types from the extensions are supported because the behavior is not conformant. With this change, the developers can use atomic types from the extensions if they are supported without enabling the pragma because disabling the pragma doesn't do anything useful but only prevents the use of already available identifiers for types and issues extra diagnostics. This makes semantics consistent with the atomic functions that are also available when the extension is supported without any need for the pragma.
Apr 23 2021
Thanks, I believe this goes in the right direction.
Feels appropriate to remove this indeed.
Apr 22 2021
Reasoning and strategy looks sensible to me. I'm not formally approving the patch because I feel I'm not familiar enough with the code base to do that.
Mar 2 2021
LGTM, thanks for the update.
Feb 24 2021
Some minor comments.
Some relatively minor comments, overall direction seems good to me.
Feb 23 2021
looks appropriate to me.
Feb 16 2021
I feel this would be a valuable addition, indeed. Only a minor question from me.
Feb 15 2021
Feb 11 2021
Feb 9 2021
LGTM, thanks. All the comments seem to be addressed.
Feb 8 2021
Nice refactoring & fix! LGTM. I suppose Stuart's comment about moving typeNameRef can be addressed when pushing.
Jan 27 2021
Right, thanks for the explanations. They make sense.
Looks sensible to me.
Jan 8 2021
LGTM
Jan 7 2021
Looks good overall.
Jan 6 2021
Thanks for the update.
LGTM, except for a minor typo. Otherwise my comments have been addressed. Thanks.
LGTM, just one question: I see in the other review you updated clang/test/SemaOpenCL/extension-version.cl. Do you need to do the same here?
LGTM
Dec 31 2020
Thanks for the update. Looks good overall. A minor question about backticks. I'm no native English speaker, so I would recommend a second review from someone else too.
lgtm, thanks
Dec 30 2020
Looks good, only a few typos and minor comments.
Nov 26 2020
When reading the documentation [1] for -cl-ext (which I've never used so far), I've noticed nothing is said about non-standard configurations (such as disabling cl_khr_depth_images with CL2.0). Quickly testing this shows that options can be specified to produce non-standard behaviour, as shown by https://godbolt.org/z/1Yz1Md.
Nov 13 2020
LGMT as it reduces the divergence in compilation flows. I let @Anastasia, or someone else more familiar with the codebase, give the final approval though.
Nov 9 2020
Oct 30 2020
Oct 29 2020
I've put up https://reviews.llvm.org/D90385 as a fix for this issue.
A follow up on https://reviews.llvm.org/D60193.
Oct 22 2020
Oct 20 2020
I don't want to stop the wider discussion, that being said I think I've addressed the comment regarding the content of this PR. Let me know if the latest version is fine or needs further addressing. Thanks.
Oct 16 2020
Keep cl_khr_byte_addressable_store and cles_khr_int64 out of this PR. Add more details on the extensions being removed in the commit message.
Oct 15 2020
Addressed comments.
Oct 14 2020
May 11 2020
LGTM
May 7 2020
Thanks for your clarifications and updates. Just one tiny question about a test file, but otherwise LGTM.
Oct 17 2019
Alright, thanks for the explanation. LGTM then.
Shouldn't this page be referenced from clang/docs/index.rst?
Oct 16 2019
Sep 25 2019
Sep 24 2019
I don't have PYTHONWARNINGS set in my env. Maybe ninja invoke the script using python -Wignore write_cmake_config.py or something? It's quite worrisome that Python runtime can exhibit different behaviours without apparent reasons. :/
/usr/bin/env python --version Python 3.7.0
I'm afraid I can't give you the exact log we have, because we have a special integration of LLVM downstream and don't directly rely on the usual cmake/gn/... workflow. I'm sorry about that.
You need Python 3.7.4 or later. The warnings are emitted only starting from this minor release.
Sep 19 2019
Let me know what you think of this patch. Thanks.
Sep 3 2019
Aug 21 2019
I think this looks good. Maybe the tests should be extended to test auto as function return type, and if there's some special handling around decltype(auto), then it should be tested too, but I'm not sure it's actually needed here. What do you think?