[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)
Roman Lebedev via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 11 09:37:14 PDT 2020
lebedev.ri added a comment.
In D59214#1916596 <https://reviews.llvm.org/D59214#1916596>, @sammccall wrote:
> So if I understand the history here:
>
> - the `IsOMPStructuredBlock` bit on `Stmt` was added to implement `getStructuredBlock()`
> - after review, `getStructuredBlock()` doesn't use the bit
> - the bit is used in the textual AST dump, and an AST matcher (which is in turn never used in-tree)
>
> Can we have the bit back please :-) We're currently trying to add a HasErrors bit to improve error-recovery and some nodes are completely full. Using a bit on every Stmt to improve the text dump a little and provide a matcher (only relevant to OMP files) seems disproportionately expensive.
In D59214#1917106 <https://reviews.llvm.org/D59214#1917106>, @rjmccall wrote:
> If the bit is unused except for propagation, please remove it.
This review (& https://bugs.llvm.org/show_bug.cgi?id=40563 in general)
has left me so burnout as to all this openmp stuff that i never
followed-up with the actual planned usages of this bit.
The reasons why this bit is needed (i think they were spelled out here)
still apply. One obvious example is https://bugs.llvm.org/show_bug.cgi?id=41102,
where after we'd teach `ExceptionAnalyzer` to look into `CapturedStmt` / `CapturedDecl`,
we'd immediately have false-positives for these OpenMP structured blocks,
so we'd need to avoid traversing into them.
Doing that via a single bit check should be trivial, as opposed to,
i dunno, maintainting a set of all the openmp structured block statements,
and before visiting each statement seeing if it is in that set?
That kinda sounds slow.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59214/new/
https://reviews.llvm.org/D59214
More information about the cfe-commits
mailing list