[PATCH] D120568: [flang][driver] Add support for -S and implement -c/-emit-obj

[Andrzej Warzynski via Phabricator via llvm-commits]

awarzynski added inline comments.


================
Comment at: flang/lib/Frontend/FrontendActions.cpp:561
+      std::make_unique<llvm::TargetLibraryInfoImpl>(triple);
+  CodeGenPasses.add(new llvm::TargetLibraryInfoWrapperPass(*TLII));
+
----------------
schweitz wrote:
> Got jumbled around somehow. TLII is allocated at line 559-560. Since you are checking allocations with assert in other places, why not here? TLII is immediately dereferenced on line 561, without an assertion check.
Thanks for clarifying. One `assert` coming this way :)


================
Comment at: flang/lib/Optimizer/Support/FIRContext.cpp:19
 
-static constexpr const char *tripleName = "fir.triple";
+static constexpr const char *tripleName = "llvm.target_triple";
 
----------------
schweitz wrote:
> awarzynski wrote:
> > schweitz wrote:
> > > Why is this being changed? This is a FIR specific attribute.
> > The driver assumes that when compiling a Fortran file, a valid triple will be present in the generated LLVM IR file. Currently, `fir.tiple` is just dropped and the generated LLVM IR module contains no triple. For `-emit-obj`/`-S` to work, we do need to make sure that a valid triple is actually there.
> > 
> > We could add logic to translate `fir.triple` to `llvm.target_triple` (in the FIR --> LLVM MLIR translation layer), but why bother if it's not used anywhere? Instead, Flang could use `llvm.target_triple` from the very beginning. This way, no extra functionality is needed (`llvm.target_triple` is preserved when translating from FIR to LLVM MLIR and then included in the generated LLVM IR module).
> > 
> > AFAIK, `fir-triple` is never used anywhere. In fact, it's already been removed from [[ https://github.com/flang-compiler/f18-llvm-project/blob/fir-dev/flang/lib/Optimizer/Support/FIRContext.cpp#L19 | fir-dev ]].
> > 
> Thanks for the update. I'm glad to see MLIR decided to use my module attribute trick here. :)
> 
> Given this, there is no reason to "block copy" the name of the attribute over to FIR. Just use the `mlir::LLVM::LLVMDialect::getTargetTripleAttrName()` method. The FIR attribute can then be retired and removed along with the setter and getter.
I wasn't aware of `getTargetTripleAttrName`, thanks! I will use that instead.

 As for `setTargetTriple` and `getTargetTriple`, we'd need a bit more work to remove them. I suggest doing that in a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120568