[PATCH] D82693: [flang] Upstream two FIR transformation passes

Eric Schweitz via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 29 10:48:17 PDT 2020


schweitz marked an inline comment as done.
schweitz added a comment.

In D82693#2120288 <https://reviews.llvm.org/D82693#2120288>, @jdoerfert wrote:

> In the passes files there are 6 pass creators declared. I failed to find all the definitions in this patch.
>  It seems sensible to split this into multiple commits. I would also recommend tests for new functionality.
>
> [EDIT: Took too long to write my comments, seems abandoned now]


This is the nature of this process we're on. We're being asked to upstream tiny bits with each diff, the smaller the better. The reality is that however one slices it, the code will always be interrelated. In order to keep the diffs small, some sort of compromise must be made.



================
Comment at: flang/lib/Optimizer/Transforms/RewriteLoop.cpp:84
+    if (onetrip)
+      rewriter.create<mlir::BranchOp>(loc, firstBlock, ArrayRef<mlir::Value>());
+    else
----------------
jdoerfert wrote:
> There seems little reason to commit dead code.
I agree that this can be cleaned up.

I disagree in that we will be upstreaming code with TODOs in the short term. The only other approach is to wait until flang is a final polished product, which would make it awkward for the LLVM community as a whole to contribute.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82693





More information about the llvm-commits mailing list