awarzynski added inline comments. Herald added a subscriber: sunshaoce.
================ Comment at: clang/include/clang/Driver/Options.td:5056-5060 +def fstack_arrays : Flag<["-"], "fstack-arrays">, Group<f_Group>, + HelpText<"Attempt to allocate array temporaries on the stack, no matter their size">; +def fno_stack_arrays : Flag<["-"], "fno-stack-arrays">, Group<f_Group>, + HelpText<"Allocate array temporaries on the heap (default)">; + ---------------- We should avoid duplicating options like this and use multiclasses instead. For example, see how [[ https://github.com/llvm/llvm-project/blob/b6ceadf3b663427f3cc233bbfdb5e35017cabd9e/clang/include/clang/Driver/Options.td#L6464-L6467 | debug_pass_manager ]] is defined. ================ Comment at: flang/docs/FlangDriver.md:573 +`-O3 -ffast-math -fstack-arrays -fno-semantic-interposition`. `-fno-semantic-interposition` is not used because clang does not enable this as part of `-Ofast` as the default behaviour is similar. ---------------- [nit] "Clang" is a sub-project. It's not clear what "clang" would be - also a sub-project or the binary? ================ Comment at: flang/include/flang/Tools/CLOptions.inc:159 inline void createDefaultFIROptimizerPassPipeline( - mlir::PassManager &pm, llvm::OptimizationLevel optLevel = defaultOptLevel) { + mlir::PassManager &pm, bool stackArrays = false, llvm::OptimizationLevel optLevel = defaultOptLevel) { // simplify the IR ---------------- CLANG-FORMAT-ME :) Same for other long lines here. ================ Comment at: flang/include/flang/Tools/CLOptions.inc:216 inline void createMLIRToLLVMPassPipeline( - mlir::PassManager &pm, llvm::OptimizationLevel optLevel = defaultOptLevel) { + mlir::PassManager &pm, bool stackArrays = false, llvm::OptimizationLevel optLevel = defaultOptLevel) { // Add default optimizer pass pipeline. ---------------- [nit] To me, it would make more sense to put `stackArrays` at the end. `optLevel`is a more powerful flag. ================ Comment at: flang/test/Transforms/stack-arrays.f90:3 +! We have to check llvm ir here to force flang to run the whole mlir pipeline +! this is just to check that -fstack-arrays enables the stack-arrays pass so ---------------- Also, I don't quite follow this comment: > We have to check llvm ir here to force flang to run the whole mlir pipeline Why is checking LLVM IR going to force Flang to run anything? ================ Comment at: flang/test/Transforms/stack-arrays.f90:6 +! only check the first example +! RUN: %flang_fc1 -emit-llvm -o - -fstack-arrays %s | FileCheck --check-prefix=CHECK-LLVM %s + ---------------- ================ Comment at: flang/tools/bbc/bbc.cpp:276 // Add O2 optimizer pass pipeline. - fir::createDefaultFIROptimizerPassPipeline(pm, llvm::OptimizationLevel::O2); + fir::createDefaultFIROptimizerPassPipeline(pm, false, + llvm::OptimizationLevel::O2); ---------------- Similar suggestion below. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140972/new/ https://reviews.llvm.org/D140972 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits