[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