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