- User Since
- Apr 5 2021, 7:22 AM (70 w, 3 d)
Apr 22 2022
Feb 8 2022
Thanks, @aaron.ballman ! I have applied the new suggestions.
Feb 7 2022
- Replaces isExprArg with isArgMemberExprHolder which returns true if the "argument member" can hold one or more expressions.
- Renames checkStringLiteralExpr to checkASCIIStringLiteralExpr to better convey the intended semantics.
- Reverts changes to HandleAnnotateAttr and removes HandleAnnotationAttr, moving final-args checks of expressions into instantiateDependentAnnotationAttr.
- Removes nonsensical comment.
Added a check to prevent duplicate "empty" constructors, i.e. if there are only optional arguments such as a single variadic argument.
Feb 4 2022
Added missing isVariadicExprArgument function.
Added check restricting variadic expression arguments to the last argument of attributes set to support parameter packs.
Added release notes.
Feb 3 2022
Adjusted for comments and introduced common handling for creating attributes with delayed arguments.
Feb 1 2022
Added tests for redeclarations and template specialization using clang::annoate with packs.
Jan 31 2022
Adds check and diagnostic for no arguments in annotate attribute.
Removed unnecessary test for arguments.
Jan 27 2022
Upon offline sync with @aaron.ballman and @erichkeane I have changed the strategy of this patch to lift the restriction of pack expansion not spanning argument boundaries. This is implemented in clang::annotate by delaying arguments to after template-instantiation if any of the arguments are value dependent.
Jan 26 2022
Removing top-level const and adjusting style.
Jan 21 2022
Jan 20 2022
Thank you, @aaron.ballman ! I will update the revision in a moment with some of the suggested changes.
Jan 12 2022
I have made the changes suggested in my previous comment. This makes significantly more changes to the parsing of attribute arguments as the old path was needed for attributes that allow both expressions and types. It also required some new controlling arguments for ParseExpressionList.
Dec 21 2021
I agree that reusing existing parser logic for this would be preferable, but as @aaron.ballman mentions Parser::ParseSimpleExpressionList does not give us the parameter pack expansion parsing we need. We could potentially unify it with the logic from https://github.com/llvm/llvm-project/blob/main/clang/lib/Parse/ParseExpr.cpp#L3059 but on the other hand there could be potential benefits to using Parser::ParseExpressionList instead.
Dec 9 2021
Dec 8 2021
Dec 7 2021
The new revision does the following:
Dec 3 2021
Thank you both for the reviews! Consensus seems to be having support for pack expansion at argument level for now and let more complicated logic be added when there is an actual need. I have applied these changes as I understood them and have added/adjusted the requested test cases. Please have a look and let me know what you think. 😄
Dec 2 2021
Nov 23 2021
Nov 18 2021
Absolutely! Thank you for making this change.
Jul 30 2021
BTW, I think it may be a good time for you to ask for LLVM commit access. llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access
Adjusted for feedback.
Made sure the PTX73 and PTX74 macros are popped correctly.
Jul 29 2021
Jul 15 2021
LGTM! Are the test failures related to this, you reckon?
Jul 13 2021
Thank you for addressing my concerns. I am happy with the changes. Great work!
Jul 12 2021
Good stuff! Thanks for adding this and adjusting the test generator. I have requested some minor changes, though nothing critical. Are the test failures related to these changes?
Sorry for the late response. Looks like you have handled the issues and more in your patch. Thank you for fixing my blunders. 😄
Jun 28 2021
Added comment about variant ordering.
Jun 25 2021
@tra Thank you for the quick response! I agree with your comments and have made changes accordingly.
Adjusted for comments and fixed formatting issues.
Jun 24 2021
May 13 2021
@tra Thanks a ton for the review! This is my first LLVM patch so I only know as much as the Code Review documentation tells me. Is there a process for chasing up additional reviews?
Apr 22 2021
Do you know if any existing code already uses the __nvvm_* builtins for cp.async? In other words, does nvcc provide them already or is it something we're free to name as we wish? I do not see any relevant intrinsics mentioned in NVVM IR spec: https://docs.nvidia.com/cuda/nvvm-ir-spec/index.html and I don't think NVCC's builtins are publicly documented anywhere.
Apr 21 2021
- Removed integer type from builtin and intrinsic names.
- Signedness in builtin and intrinsic names moved to operator name, i.e. umin and umax.
- Removed redundant addition variant.
Apr 20 2021
Apr 13 2021
Apr 9 2021
@tra Thank you for the feedback! I think I see what you're getting at, but I am not quite understanding how it would work for these builtins and intrinsics. I have added some comments to the corresponding feedback about my confusion and/or concerns.