[PATCH] D92533: [flang][openacc] Add missing loop construct restriction and validity tests
sameeran joshi via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 4 09:32:14 PST 2020
sameeranjoshi added a comment.
The tests look good, just a few comments/queries?
I am really not familiar with canonicalization, but I could grasp what's defined in this patch.
================
Comment at: flang/lib/Semantics/canonicalize-acc.cpp:68
+ if (outer->IsDoConcurrent())
+ return; // Tile is not allowed on DO CONURRENT
for (const parser::DoConstruct *loop{&*outer}; loop && tileArgNb > 0;
----------------
If that's a restriction why is this not in the form of a semantic error message?
================
Comment at: flang/lib/Semantics/canonicalize-acc.cpp:87
+ // Check constraint on line 1835 in Section 2.9
+ // A tile and collapse clause may not appear on loop that is associated with
----------------
Which standard ?
There are patches for 3.1 up gradation, will line numbers change in that case?
Is 3.0 assumed when no reference?
================
Comment at: flang/lib/Semantics/canonicalize-acc.cpp:99
+ for (const auto &clause : accClauseList.v) {
+ if (std::get_if<parser::AccClause::Collapse>(&clause.u) ||
+ std::get_if<parser::AccClause::Tile>(&clause.u)) {
----------------
A C++ beginners query -
Why use `std::get_if` over `std::holds_alternative` if we are not utilizing the return pointer?
================
Comment at: flang/lib/Semantics/canonicalize-acc.cpp:102
+ messages_.Say(beginLoopDirective.source,
+ "TILE and COLLAPSE clause may not appear on loop construct "
+ "associated with DO CONCURRENT"_err_en_US);
----------------
`nit`:Was that `or` instead of `and` ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92533/new/
https://reviews.llvm.org/D92533
More information about the llvm-commits
mailing list