[Flang] Add user option -funderscoring/-fnounderscoring to control trailing underscore added to external names

[madanial]

Summary

This patch adds user option -funderscoring/-fnounderscoring to control the trailing underscore being appended to external names (e.g. procedure names, common block names). The option in gfortran is documented in https://gcc.gnu.org/onlinedocs/gfortran/Code-Gen-Options.html.

[clementval]

You need to update the driver-help tests.

flang/include/flang/Tools/CLOptions.inc

194

Please follow the same formatting in this file.

213

Doesn't follow the formatting here.

[awarzynski]

Hi @madanial , thanks for posting this!

This patch adds user option -funderscoring/-fnounderscoring which behaves similar to the gfortran option be enabling/disabling the ExternalNameConversionPass

I don't quite understand what this option is for and it's hard to deduce from the patch. Please, could you add a link to some documentation? And tests.

-Andrzej

[kkwli0]

In D140795#4022493, @awarzynski wrote:

Hi @madanial , thanks for posting this!

This patch adds user option -funderscoring/-fnounderscoring which behaves similar to the gfortran option be enabling/disabling the ExternalNameConversionPass

I don't quite understand what this option is for and it's hard to deduce from the patch. Please, could you add a link to some documentation? And tests.

-Andrzej

@awarzynski Thanks for the review. @madanial will not be available for the next few weeks. I will try to address some review comments while he is away.

The purpose of this option is to control the trailing underscore being appended to external names (e.g. procedure names, common block names). The option in gfortran is documented in https://gcc.gnu.org/onlinedocs/gfortran/Code-Gen-Options.html.

However, I don't think the patch does what we want. Given a procedure name foo, the -fno-underscoring option will give _QPfoo instead of foo. We will look into it.

However, I don't think the patch does what we want. Given a procedure name foo, the -fno-underscoring option will give _QPfoo instead of foo.

[awarzynski]

In D140795#4031392, @kkwli0 wrote:

The purpose of this option is to control the trailing underscore being appended to external names (e.g. procedure names, common block names). The option in gfortran is documented in https://gcc.gnu.org/onlinedocs/gfortran/Code-Gen-Options.html.

Thanks for this explanation - much appreciated! Could this short description be added to the summary? Also, could you add a note stating whether the semantics of this option in Flang are consistent with GFortran. Ta!

However, I don't think the patch does what we want. Given a procedure name foo, the -fno-underscoring option will give _QPfoo instead of foo. We will look into it.

Names in Flang are mangled at the FIR level. I couldn't find any documentation for this, but the ExternalNameConversion pass could be helpful.

[kkwli0]

In D140795#4033559, @awarzynski wrote:

However, I don't think the patch does what we want. Given a procedure name foo, the -fno-underscoring option will give _QPfoo instead of foo. We will look into it.

Names in Flang are mangled at the FIR level. I couldn't find any documentation for this, but the ExternalNameConversion pass could be helpful.

@awarzynski Thanks for the info. We intend to make use of ExternalNameConversion for this purpose. However, we are not sure how a compiler option can control the behavior in ExternalNameConversion. Any pointer to how other passes do it is appreciated.

[clementval]

You should have a look at https://mlir.llvm.org/docs/PassManagement/#instance-specific-pass-options

[madanial]

Implementing functionality to control just the trailing underscore for external names to achieve similar functionality to the option in gfortran, as well as rebase and address review comments.

[clementval]

Small suggestion

flang/lib/Optimizer/Transforms/ExternalNameConversion.cpp

41–45 ↗

(On Diff #492312)

To avoid new copy

[madanial]

Addressing review comment

[clementval]

Can you also add test in flang/test/Fir/external-mangling.fir?

[awarzynski]

Driver changes LGTM, thanks! I will defer to others for changes in other areas.

[madanial]

Rebase, as well as bug fix relating to the ExternalNameConversion pass option that was exposed by the requested fir-opt test case. The pass option was being overwritten by default value whenever it was called through fir-opt.

[clementval]

flang/test/Fir/external-mangling.fir

36 ↗

(On Diff #496027)

You should check at least the character after it because here the check line would also succeed with an underscore.

39–40 ↗

(On Diff #496027)

Same here.

85–88 ↗

(On Diff #496027)

Same here.

[madanial]

Addressing the review comments relating to the test case. Thanks for the catch!

[clementval]

You still have clang-format issues in your patch. Can you update that.

[madanial]

clang-format issue in ExternalNameConversion.cpp

[clementval]

The pre-commit check is still failing with these tests:

Flang :: Fir/alloc.fir

Flang :: Fir/embox.fir
Flang :: Fir/optional.fir
Flang :: Fir/rebox.fir
Flang :: Lower/common-block.f90
Flang :: Lower/forall/character-1.f90

[madanial]

Bug Fix for failing test cases

[clementval]

You still have style mismatch in flang/include/flang/Tools/CLOptions.inc. This file is likely not run through clang-format so that's why.

flang/include/flang/Tools/CLOptions.inc

196

211–213

218

[madanial]

addressing style mismatch in flang/include/flang/Tools/CLOptions.inc

[clementval]

LGTM. Thanks for working on this.

[madanial]

Thanks for the review.

clang/include/clang/Driver/Options.td

 defm real_8_real_16 : BooleanFFlag<"real-8-real-16">, Group<gfortran_Group>;
 defm real_8_real_4 : BooleanFFlag<"real-8-real-4">, Group<gfortran_Group>;
 defm realloc_lhs : BooleanFFlag<"realloc-lhs">, Group<gfortran_Group>;
 defm recursive : BooleanFFlag<"recursive">, Group<gfortran_Group>;
 defm repack_arrays : BooleanFFlag<"repack-arrays">, Group<gfortran_Group>;
 defm second_underscore : BooleanFFlag<"second-underscore">, Group<gfortran_Group>;
 defm sign_zero : BooleanFFlag<"sign-zero">, Group<gfortran_Group>;
 defm stack_arrays : BooleanFFlag<"stack-arrays">, Group<gfortran_Group>;
-defm underscoring : BooleanFFlag<"underscoring">, Group<gfortran_Group>;
 defm whole_file : BooleanFFlag<"whole-file">, Group<gfortran_Group>;
 
 // C++ SYCL options
 def fsycl : Flag<["-"], "fsycl">, Flags<[NoXarchOption, CoreOption]>,
   Group<sycl_Group>, HelpText<"Enables SYCL kernels compilation for device">;
 def fno_sycl : Flag<["-"], "fno-sycl">, Flags<[NoXarchOption, CoreOption]>,
   Group<sycl_Group>, HelpText<"Disables SYCL kernels compilation for device">;
 
def fintrinsic_modules_path : Separate<["-"], "fintrinsic-modules-path">,  Group<f_Group>, MetaVarName<"<dir>">,
   HelpText<"Specify where to find the compiled intrinsic modules">,
   DocBrief<[{This option specifies the location of pre-compiled intrinsic modules,
   if they are not in the default location expected by the compiler.}]>;
 
 defm backslash : OptInFC1FFlag<"backslash", "Specify that backslash in string introduces an escape character">;
 defm xor_operator : OptInFC1FFlag<"xor-operator", "Enable .XOR. as a synonym of .NEQV.">;
 defm logical_abbreviations : OptInFC1FFlag<"logical-abbreviations", "Enable logical abbreviations">;
 defm implicit_none : OptInFC1FFlag<"implicit-none", "No implicit typing allowed unless overridden by IMPLICIT statements">;
+defm underscoring : OptInFC1FFlag<"underscoring", "Appends one trailing underscore to external names">;
 
 def fno_automatic : Flag<["-"], "fno-automatic">, Group<f_Group>,
   HelpText<"Implies the SAVE attribute for non-automatic local objects in subprograms unless RECURSIVE">;
 
 } // let Flags = [FC1Option, FlangOption, FlangOnlyOption]
 
 def J : JoinedOrSeparate<["-"], "J">,
   Flags<[RenderJoined, FlangOption, FC1Option, FlangOnlyOption]>,

clang/lib/Driver/ToolChains/Flang.cpp

Args.AddAllArgs(CmdArgs,
                    options::OPT_I, options::OPT_cpp, options::OPT_nocpp});
 }
 
 void Flang::addOtherOptions(const ArgList &Args, ArgStringList &CmdArgs) const {
   Args.AddAllArgs(CmdArgs,
                   {options::OPT_module_dir, options::OPT_fdebug_module_writer,
                    options::OPT_fintrinsic_modules_path, options::OPT_pedantic,
                    options::OPT_std_EQ, options::OPT_W_Joined,
-                   options::OPT_fconvert_EQ, options::OPT_fpass_plugin_EQ});
+                   options::OPT_fconvert_EQ, options::OPT_fpass_plugin_EQ,
+                   options::OPT_funderscoring, options::OPT_fno_underscoring});
 }
 
 void Flang::addPicOptions(const ArgList &Args, ArgStringList &CmdArgs) const {
   // ParsePICArgs parses -fPIC/-fPIE and their variants and returns a tuple of
   // (RelocationModel, PICLevel, IsPIE).
   llvm::Reloc::Model RelocationModel;
   unsigned PICLevel;
   bool IsPIE;

flang/include/flang/Frontend/CodeGenOptions.def

 CODEGENOPT(OptimizationLevel, 2, 0) ///< The -O[0-3] option specified.
 
 CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
                                    ///< pass manager.
 
 CODEGENOPT(PICLevel, 2, 0) ///< PIC level of the LLVM module.
 CODEGENOPT(IsPIE, 1, 0) ///< PIE level is the same as PIC Level.
 
+CODEGENOPT(Underscoring, 1, 1)
 ENUM_CODEGENOPT(RelocationModel, llvm::Reloc::Model, 3, llvm::Reloc::PIC_) ///< Name of the relocation model to use.
 
 #undef CODEGENOPT
 #undef ENUM_CODEGENOPT

flang/include/flang/Tools/CLOptions.inc

inline void createDefaultFIROptimizerPassPipeline(
   pm.addPass(mlir::createConvertSCFToCFPass());
 
   pm.addPass(mlir::createCanonicalizerPass(config));
   pm.addPass(fir::createSimplifyRegionLitePass());
   pm.addPass(mlir::createCSEPass());
 }
 
 #if !defined(FLANG_EXCLUDE_CODEGEN)
-inline void createDefaultFIRCodeGenPassPipeline(mlir::PassManager &pm) {
+inline void createDefaultFIRCodeGenPassPipeline(mlir::PassManager &pm, bool Underscoring = 1) {
   fir::addBoxedProcedurePass(pm);
   pm.addNestedPass<mlir::func::FuncOp>(
       fir::createAbstractResultOnFuncOptPass());
   pm.addNestedPass<fir::GlobalOp>(
       fir::createAbstractResultOnGlobalOptPass());
   fir::addCodeGenRewritePass(pm);
   fir::addTargetRewritePass(pm);
+  if (Underscoring)
     fir::addExternalNameConversionPass(pm);
   fir::addFIRToLLVMPass(pm);
 }
 
 /// Create a pass pipeline for lowering from MLIR to LLVM IR
 ///
 /// \param pm - MLIR pass manager that will hold the pipeline definition
 /// \param optLevel - optimization level used for creating FIR optimization
 ///   passes pipeline
-inline void createMLIRToLLVMPassPipeline(
-    mlir::PassManager &pm, llvm::OptimizationLevel optLevel = defaultOptLevel) {
+inline void createMLIRToLLVMPassPipeline(mlir::PassManager &pm,
+    llvm::OptimizationLevel optLevel = defaultOptLevel, bool Underscoring = 1) {
   // Add default optimizer pass pipeline.
   fir::createDefaultFIROptimizerPassPipeline(pm, optLevel);
 
   // Add codegen pass pipeline.
-  fir::createDefaultFIRCodeGenPassPipeline(pm);
+  fir::createDefaultFIRCodeGenPassPipeline(pm, Underscoring);
 }
 #undef FLANG_EXCLUDE_CODEGEN
 #endif
 
 } // namespace fir

flang/lib/Frontend/CompilerInvocation.cpp

if (PICLevel > 2)
           << args.getLastArg(clang::driver::options::OPT_pic_level)
                  ->getAsString(args)
           << PICLevel;
 
     opts.PICLevel = PICLevel;
     if (args.hasArg(clang::driver::options::OPT_pic_is_pie))
       opts.IsPIE = 1;
   }
+  if (args.hasFlag(clang::driver::options::OPT_fno_underscoring,
+                   clang::driver::options::OPT_funderscoring, false)) {
+    opts.Underscoring = 0;
+  }
 }
 
 /// Parses all target input arguments and populates the target
 /// options accordingly.
 ///
 /// \param [in] opts The target options instance to update
 /// \param [in] args The list of input arguments (from the compiler invocation)
 static void parseTargetArgs(TargetOptions &opts, llvm::opt::ArgList &args) {

flang/lib/Frontend/FrontendActions.cpp

 // Lower the previously generated MLIR module into an LLVM IR module
 void CodeGenAction::generateLLVMIR() {
   assert(mlirModule && "The MLIR module has not been generated yet.");
 
   CompilerInstance &ci = this->getInstance();
   auto opts = ci.getInvocation().getCodeGenOpts();
   llvm::OptimizationLevel level = mapToLevel(opts);
 
+  auto Underscoring = opts.Underscoring;
   fir::support::loadDialects(*mlirCtx);
   fir::support::registerLLVMTranslation(*mlirCtx);
 
   // Set-up the MLIR pass manager
   mlir::PassManager pm(mlirCtx.get(), mlir::OpPassManager::Nesting::Implicit);
 
   pm.addPass(std::make_unique<Fortran::lower::VerifierPass>());
   pm.enableVerifier(/*verifyPasses=*/true);
 
   // Create the pass pipeline
-  fir::createMLIRToLLVMPassPipeline(pm, level);
+  fir::createMLIRToLLVMPassPipeline(pm, level, Underscoring);
   mlir::applyPassManagerCLOptions(pm);
 
   // run the pass manager
   if (!mlir::succeeded(pm.run(*mlirModule))) {
     unsigned diagID = ci.getDiagnostics().getCustomDiagID(
         clang::DiagnosticsEngine::Error, "Lowering to LLVM IR failed");
     ci.getDiagnostics().Report(diagID);
   }