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

[Eric Schweitz via Phabricator via cfe-commits]

schweitz added inline comments.
Herald added a project: All.


================
Comment at: flang/lib/Frontend/FrontendActions.cpp:561
+      std::make_unique<llvm::TargetLibraryInfoImpl>(triple);
+  CodeGenPasses.add(new llvm::TargetLibraryInfoWrapperPass(*TLII));
+
----------------
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.


================
Comment at: flang/lib/Optimizer/Support/FIRContext.cpp:19
 
-static constexpr const char *tripleName = "fir.triple";
+static constexpr const char *tripleName = "llvm.target_triple";
 
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120568