[PATCH] D118983: [Flang] Add support for lowering the goto statement

[Andrzej Warzynski via Phabricator via llvm-commits]

awarzynski added inline comments.


================
Comment at: flang/lib/Lower/Bridge.cpp:136
+  Fortran::lower::pft::Evaluation &getEval() {
+    assert(evalPtr);
+    return *evalPtr;
----------------
[nit] Could you add a message here?


================
Comment at: flang/lib/Lower/Bridge.cpp:198-200
+    builder->setInsertionPointToStart(newBlock);
+    if (blockIsUnterminated())
+      builder->setInsertionPointToEnd(newBlock);
----------------
Otherwise you might be setting the insertion point more than once, right?


================
Comment at: flang/lib/Lower/Bridge.cpp:199
+    builder->setInsertionPointToStart(newBlock);
+    if (blockIsUnterminated())
+      builder->setInsertionPointToEnd(newBlock);
----------------
Wouldn't `genFIRBranch(newBlock)` above create a terminator? So that this `if` condition is always `true`?


================
Comment at: flang/lib/Lower/Bridge.cpp:245-254
+  /// Create global blocks for the current function.  This eliminates the
+  /// distinction between forward and backward targets when generating
+  /// branches.  A block is "global" if it can be the target of a GOTO or
+  /// other source code branch.  A block that can only be targeted by a
+  /// compiler generated branch is "local".  For example, a DO loop preheader
+  /// block containing loop initialization code is global.  A loop header
+  /// block, which is the target of the loop back edge, is local.  Blocks
----------------
I don't fully follow this comment. For example, it mentions "global" and "local" blocks, but there's no mention of "global" or "local" in the actual code. Perhaps that's just me?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118983/new/

https://reviews.llvm.org/D118983