User Details
- User Since
- Apr 14 2020, 10:41 AM (164 w, 2 d)
Jan 4 2023
Nov 18 2021
Thanks.
LGTM.
Jan 21 2021
Capitalized clauses in message.
Summary of changes:
Jan 20 2021
We are slowly trying to replace the throwaway driver f18. That's actually a good progress.
Thank you for the progress @awarzynski @FarisRehman
I would wait for others to approve as this patch touches the core Fortran parts.
Jan 19 2021
Looks good.
Ping.
Jan 18 2021
Ok a clang version 8.0.1 based build passes.
It's seems compiler version related issue.
BTW, is there a way to mitigate or conform such compiler version/build related issues?
LGTM.
Thank you.
LGTM.
Agreed. @awarzynski. Thank you for clarification.
Jan 17 2021
Thank you for working on this.
It build and tests successfully.
Please wait for other reviewers to accept.
This was a much needed change. Thank you for the refactoring.
One of the typical and most common problems was FindClause did not work as expected given that CheckAllowed was correctly defined for the clauses.
Jan 14 2021
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.
Jan 13 2021
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.
Jan 12 2021
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.
Jan 11 2021
LGMT. Thanks for cleaning that up.
Looks good.
Jan 10 2021
Jan 9 2021
Jan 7 2021
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.
Jan 5 2021
Jan 4 2021
Mostly nit comments.
Thanks and LGMT.
Jan 3 2021
Dec 22 2020
This patch was co-authored by @valentime. I forgot to update the commit message when pushing to main, apologies.
Dec 21 2020
Thank you!
That was much needed to extract into a fixture, given a few upcoming reviews.
Dec 18 2020
Remove unnecessary variable mayBeName.
Merge ResolveName into ResolveOmpName.
+1 for finding the issue.
Thanks for the efforts and LGMT.
Dec 17 2020
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.