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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
