User Details
- User Since
- Apr 14 2020, 10:41 AM (39 w, 2 d)
Yesterday
Thank you for the patch.
It was pending since many days.
The changes look good to me from the matching point w.r.t the linked patch.
I couldn't understand still the rational for *.cpp.inc to *.inc for both OMP and OACC, could you please point why was that needed?
Thank you.
Thank you for the patch.
I did get an email from the bot with a weird text Control flow escapes from \xcb\x1c\xda\xff\xff\nexpect at 16: Control flow escapes from PARALLEL\n'
I had a build of gcc 9.2 and the tried to check with the recent pull from main and all tests seem to work.
It was pointed[1] by @awarzynski that the issue was specifically due to clang-10.
Wed, Jan 13
Ideally in the first look the checks would fit better for ordered construct
inside a single function CheckDependOnOrderedDir so we have them at a
single place , unfortunately it's harder to implement it in above function
considering that the information to differentiate between a
OmpBlockDirective and OmpSimpleStandaloneDirective is unavailable.
Tue, Jan 12
Tweak comments as suggested.
These are great catches, thank you.
I was wondering since the merge of task_reduction why were the tests failing and did kept this on hold for a while since then.
Thanks again. :)
Summary of changes:
- Simplify the test as those checks were already covered.
- Update OMP.td to specifiy to add 5.0 standard for the clauses.
Summary of changes:
- Handle depend(source) and depend(sink:vec) separately.
- Handle one more restriction from standard[5.0][2.17.9] which was needed with this patch. Either depend(sink:vec) clauses or depend(source) clauses may appear on an ordered construct, but not both.
Mon, Jan 11
LGMT. Thanks for cleaning that up.
Looks good.
Sun, Jan 10
Sat, Jan 9
Thu, Jan 7
Summary of changes:
- Fix core dump in ordered construct due to missing tree node visit
for OpenMPSimpleStandaloneConstruct as ordered with a depend falls under it.
- Fix internal error in name resolution of depend clause.
Tue, Jan 5
Mon, Jan 4
Mostly nit comments.
Thanks and LGMT.
Sun, Jan 3
Tue, Dec 22
Co-authored-by: Valentin Clement <clementval@gmail.com>
Mon, Dec 21
Thank you!
That was much needed to extract into a fixture, given a few upcoming reviews.
Fri, Dec 18
Remove unnecessary variable mayBeName.
Merge ResolveName into ResolveOmpName.
+1 for finding the issue.
Thanks for the efforts and LGMT.
Thu, Dec 17
Address review comments.
LGTM.
Please take care of the remaining nits comments before merging.
Thank you @tskeith for pointers.
Addresses one of the comments from
https://reviews.llvm.org/D88655#2356685
Please suggest.
Wed, Dec 16
Dec 16 2020
Try to simplify comments.
Thanks for patch.
A few comments below.
Thanks for the upgrade.
A comment below.
Thanks for the patch.
A few comments below.
Dec 15 2020
Fix newlines.
Thank you for reviewing.
Update OmpReductionClause to use OmpObjectList.
Make OmpTaskReductionClause depend on OmpReductionClause.
This change was needed to create consistency among the reduction clauses.
Dec 14 2020
Dec 13 2020
Looks good from my end.
Thank you.
Dec 11 2020
Dec 10 2020
Add a clarification comment.
change test name.
Change title.