Add PFT tests #6
Conversation
Note: This commit does not reflect an actual work log, it is a feature based split of the changes done in the FIR experimental branch. The related work log can be found in the commits between: schweitzpgi@8c320e3 and: schweitzpgi@9b9ea05
The ASTBuilder structure is a temporary data structure that is meant to be built from the parse tree just before lowering to FIR and that will be deleted just afterwards. It is not meant to perfrom optimization analysis and transformations. It only provides temporary information, such as label target information or parse tree parent nodes, that is meant to be used to lower the parse tree structure into FIR operations. A pretty printer is available to visualize this data structure. Note: This commit does not reflect an actual work log, it is a feature based split of the changes done in the FIR experimental branch. The related work log can be found in the commits between: 864898c and 137c23d
Review: flang-compiler#959 Fixes file names in LLVM copyright.
- change Evaluation ctor argument order to match member order - db -> bd for BlockData argument name - Use dump-parse-tree.h for the AST pretty printer.
Remove Fortran:: and Fortran::lower when not needed.
After discussions, it turns out this change is not needed.
This patch is a forward integration of an issue fix that will be fixed in parallel in f18, remove when rebasing whith next f18 containing the fix.
- Add lit test to test: 1. that the PFT tree structure is as expected 2. that the PFT captures all intented nodes - Fix: FunctionLike units were added in wrong lists after a module subprogram or an internal suprogram was visited by ASTBuilder.
| if (isConstruct() && evals) { | ||
| return evals; | ||
| } | ||
| llvm_unreachable("evaluation subs is inconsistent"); |
| if (eval.isConstruct()) { | ||
| annotateEvalListCFG(*eval.getConstructEvals(), &eval); | ||
| if (auto *subs{eval.getConstructEvals()}) { | ||
| annotateEvalListCFG(*subs, &eval); |
There was a problem hiding this comment.
Was this the bug? Don't all constructs have a body (with 0 or more evals)?
There was a problem hiding this comment.
Well, there was one bug here that I did not mentioned in the commit, not sure if it came from the refactoring, the issue was that eval.isConstruct() and eval.isStmt() were not equivalent so eval.isConstruct() was not ensuring eval.getConstructEvals() was not null and that led to weird things, like visiting ConstructStmt as if they were constructs. The eval.isConstruct() is fixed, but it feels safer to test the pointer here as a user.
| Fortran::lower::annotateControl(*ast); | ||
| Fortran::lower::dumpPFT(llvm::outs(), *ast); | ||
| } else { | ||
| std::cerr << "Pre FIR Tree is NULL.\n"; |
There was a problem hiding this comment.
Ah, actually ~ so so, std::cerr is already used in this f18 file, I do not want to spearhead the change to llvm stream here (I actually tried to hide the llvm::outs() to avoid troubles here, but I then hit llvm runtime issues (I think the driver should initialized it or something) so I left it that way).
jeanPerier
force-pushed
the
jpr-mono-split
branch
from
ee469d6 to
edb0943
Compare
we can then capture with FileCheck regex to check the tree structure in a much easier and stronger
way than checking the indentation.
I think checking the address also gives us the guarantee that there was no weird copies made and that the node are indeed uniq.
I wrote a bunch of tests related to that, the high level structure is fully checked (fixed a bug on the way), I still lack some test for some action statements, I will finish this before moving this to the PR959
but I would rather have your opinion on the tests method now.