[flang-commits] [PATCH] D121171: [flang] Add ExternalNameConversionPass to flang-new pipeline

[Pat McCormick via Phabricator via flang-commits]

pmccormick added inline comments.


================
Comment at: flang/test/Lower/Intrinsics/abs.f90:2
 ! RUN: bbc -emit-fir %s -o - | FileCheck %s
 ! RUN: %flang_fc1 -emit-fir %s -o - | FileCheck %s
 
----------------
schweitz wrote:
> schweitz wrote:
> > awarzynski wrote:
> > > schweitz wrote:
> > > > Why does this test need to be run with flang?
> > > > 
> > > > We don't see the LLVM tests being driven by both opt and clang.
> > > > We don't see the LLVM tests being driven by both opt and clang.
> > > 
> > > That would require LLVM depending on Clang. Also, `opt` is for testing (and setting up) each LLVM pass individually. Clang neither needs nor supports this level of granularity. Instead, it operates at higher abstraction level, e.g. different optimisation pipelines. The situation with `flang-new` and `bbc` is very different.
> > BTW, something appears to be incorrect here.
> > 
> > Is `-emit-fir` on `flang` supposed to behave the same way as it always has with the lowering test tool, `bbc`? The //codegen// pass to convert symbols to some target for compatibility is not part of lowering and isn't reflected in the lowering output which is what `bbc -emit-fir` produces.
> > 
> > So it looks like `flang -emit-fir` and `bbc -emit-fir` have already diverged here. Is that the intention? If so, then `flang` certainly cannot replace `bbc`.
> > That would require LLVM depending on Clang. Also, `opt` is for testing (and setting up) each LLVM pass individually. Clang neither needs nor supports this level of granularity. Instead, it operates at higher abstraction level, e.g. different optimisation pipelines. The situation with `flang-new` and `bbc` is very different.
> 
> Disagree. `bbc` is a test tool that is intended to test lowering. Specifically, the conversion of the PFT, plus other data structures, to FIR. The purpose of the `flang-new` driver is to be a Fortran compiler end-to-end. Those two objectives are absolutely, positively different.
> 
> There is clearly confusion on this point. That likely stems from the fact that the only way to pickle the PFT (and other data) is to pretty print it out as Fortran. That perfect circularity makes it appear that `bbc` is trying to be something it was not intended to be.
> 
> Can we stop conflating these two purposes?
@schweitz -- thanks for the clarification on `bbc`.   I have to admit I was confused by the differences/similarities.  Its appearance in use cases that don't feel that different from the driver have probably helped add to the confusion (at least mine! ;-).

Should we consider some way to make this distinction more clear for those new to the code base?   I suspect there is a longer history here behind `bbc` but being more specific about its intended usage might help us avoid revisiting this again down the road; especially so for those new to the code base and the supporting toolset. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121171