Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flang][Driver] Add support for -f[no-]wrapv and -f[no]-strict-overflow in the frontend #110061

Merged
merged 2 commits into from
Oct 18, 2024

Conversation

yus3710-fj
Copy link
Contributor

This patch introduces the options for integer overflow flags into Flang.
The behavior is similar to that of Clang.

@yus3710-fj yus3710-fj marked this pull request as ready for review October 3, 2024 04:15
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' flang:driver flang Flang issues not falling into any other category flang:fir-hlfir labels Oct 3, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 3, 2024

@llvm/pr-subscribers-flang-driver

@llvm/pr-subscribers-clang-driver

Author: Yusuke MINATO (yus3710-fj)

Changes

This patch introduces the options for integer overflow flags into Flang.
The behavior is similar to that of Clang.


Full diff: https://github.com/llvm/llvm-project/pull/110061.diff

8 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+7-4)
  • (modified) clang/lib/Driver/ToolChains/Flang.cpp (+11)
  • (modified) flang/include/flang/Common/LangOptions.def (+2)
  • (modified) flang/include/flang/Common/LangOptions.h (+8)
  • (modified) flang/include/flang/Lower/LoweringOptions.def (+2-3)
  • (modified) flang/lib/Frontend/CompilerInvocation.cpp (+33-3)
  • (modified) flang/test/Driver/frontend-forwarding.f90 (+2)
  • (added) flang/test/Driver/integer-overflow.f90 (+10)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 9d183ff2d69b3c..4610cd917f685d 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3451,7 +3451,8 @@ def fno_strict_aliasing : Flag<["-"], "fno-strict-aliasing">, Group<f_Group>,
 def fstruct_path_tbaa : Flag<["-"], "fstruct-path-tbaa">, Group<f_Group>;
 def fno_struct_path_tbaa : Flag<["-"], "fno-struct-path-tbaa">, Group<f_Group>;
 def fno_strict_enums : Flag<["-"], "fno-strict-enums">, Group<f_Group>;
-def fno_strict_overflow : Flag<["-"], "fno-strict-overflow">, Group<f_Group>;
+def fno_strict_overflow : Flag<["-"], "fno-strict-overflow">, Group<f_Group>,
+  Visibility<[ClangOption, FlangOption]>;
 def fno_pointer_tbaa : Flag<["-"], "fno-pointer-tbaa">, Group<f_Group>;
 def fno_temp_file : Flag<["-"], "fno-temp-file">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option, CLOption, DXCOption]>, HelpText<
@@ -3467,7 +3468,8 @@ def fno_verbose_asm : Flag<["-"], "fno-verbose-asm">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option]>,
   MarshallingInfoNegativeFlag<CodeGenOpts<"AsmVerbose">>;
 def fno_working_directory : Flag<["-"], "fno-working-directory">, Group<f_Group>;
-def fno_wrapv : Flag<["-"], "fno-wrapv">, Group<f_Group>;
+def fno_wrapv : Flag<["-"], "fno-wrapv">, Group<f_Group>,
+  Visibility<[ClangOption, FlangOption]>;
 def fobjc_arc : Flag<["-"], "fobjc-arc">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option]>,
   HelpText<"Synthesize retain and release calls for Objective-C pointers">;
@@ -3963,7 +3965,8 @@ defm strict_vtable_pointers : BoolFOption<"strict-vtable-pointers",
           "Enable optimizations based on the strict rules for"
             " overwriting polymorphic C++ objects">,
   NegFlag<SetFalse>>;
-def fstrict_overflow : Flag<["-"], "fstrict-overflow">, Group<f_Group>;
+def fstrict_overflow : Flag<["-"], "fstrict-overflow">, Group<f_Group>,
+  Visibility<[ClangOption, FlangOption]>;
 def fpointer_tbaa : Flag<["-"], "fpointer-tbaa">, Group<f_Group>;
 def fdriver_only : Flag<["-"], "fdriver-only">, Flags<[NoXarchOption]>,
   Visibility<[ClangOption, CLOption, DXCOption]>,
@@ -4232,7 +4235,7 @@ defm virtual_function_elimination : BoolFOption<"virtual-function-elimination",
   NegFlag<SetFalse>, BothFlags<[], [ClangOption, CLOption]>>;
 
 def fwrapv : Flag<["-"], "fwrapv">, Group<f_Group>,
-  Visibility<[ClangOption, CC1Option]>,
+  Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>,
   HelpText<"Treat signed integer overflow as two's complement">;
 def fwritable_strings : Flag<["-"], "fwritable-strings">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option]>,
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 98350690f8d20e..1865fd57a2d893 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -866,6 +866,17 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
     }
   }
 
+  // -fno-strict-overflow implies -fwrapv if it isn't disabled, but
+  // -fstrict-overflow won't turn off an explicitly enabled -fwrapv.
+  if (Arg *A = Args.getLastArg(options::OPT_fwrapv, options::OPT_fno_wrapv)) {
+    if (A->getOption().matches(options::OPT_fwrapv))
+      CmdArgs.push_back("-fwrapv");
+  } else if (Arg *A = Args.getLastArg(options::OPT_fstrict_overflow,
+                                      options::OPT_fno_strict_overflow)) {
+    if (A->getOption().matches(options::OPT_fno_strict_overflow))
+      CmdArgs.push_back("-fwrapv");
+  }
+
   assert((Output.isFilename() || Output.isNothing()) && "Invalid output.");
   if (Output.isFilename()) {
     CmdArgs.push_back("-o");
diff --git a/flang/include/flang/Common/LangOptions.def b/flang/include/flang/Common/LangOptions.def
index d3e1e972d1519f..1bfdba9cc2c1c7 100644
--- a/flang/include/flang/Common/LangOptions.def
+++ b/flang/include/flang/Common/LangOptions.def
@@ -20,6 +20,8 @@ LANGOPT(Name, Bits, Default)
 #endif
 
 ENUM_LANGOPT(FPContractMode, FPModeKind, 2, FPM_Fast) ///< FP Contract Mode (off/fast)
+/// signed integer overflow handling
+ENUM_LANGOPT(SignedOverflowBehavior, SignedOverflowBehaviorTy, 1, SOB_Undefined)
 
 /// Indicate a build without the standard GPU libraries.
 LANGOPT(NoGPULib  , 1, false)
diff --git a/flang/include/flang/Common/LangOptions.h b/flang/include/flang/Common/LangOptions.h
index 52a45047deb0e2..83f25cfbe26142 100644
--- a/flang/include/flang/Common/LangOptions.h
+++ b/flang/include/flang/Common/LangOptions.h
@@ -27,6 +27,14 @@ namespace Fortran::common {
 class LangOptionsBase {
 
 public:
+  enum SignedOverflowBehaviorTy {
+    // -fno-wrapv (default behavior in Flang)
+    SOB_Undefined,
+
+    // -fwrapv
+    SOB_Defined,
+  };
+
   enum FPModeKind {
     // Do not fuse FP ops
     FPM_Off,
diff --git a/flang/include/flang/Lower/LoweringOptions.def b/flang/include/flang/Lower/LoweringOptions.def
index d3f17c3f939c16..231de533fbd30a 100644
--- a/flang/include/flang/Lower/LoweringOptions.def
+++ b/flang/include/flang/Lower/LoweringOptions.def
@@ -35,9 +35,8 @@ ENUM_LOWERINGOPT(NoPPCNativeVecElemOrder, unsigned, 1, 0)
 ENUM_LOWERINGOPT(Underscoring, unsigned, 1, 1)
 
 /// If true, assume the behavior of integer overflow is defined
-/// (i.e. wraps around as two's complement). On by default.
-/// TODO: make the default off
-ENUM_LOWERINGOPT(IntegerWrapAround, unsigned, 1, 1)
+/// (i.e. wraps around as two's complement). Off by default.
+ENUM_LOWERINGOPT(IntegerWrapAround, unsigned, 1, 0)
 
 /// If true, add nsw flags to loop variable increments.
 /// Off by default.
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 2154b9ab2fbf47..c8160a4236b466 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -1109,6 +1109,24 @@ static bool parseOpenMPArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
   return diags.getNumErrors() == numErrorsBefore;
 }
 
+/// Parses signed integer overflow options and populates the
+/// CompilerInvocation accordingly.
+/// Returns false if new errors are generated.
+///
+/// \param [out] invoc Stores the processed arguments
+/// \param [in] args The compiler invocation arguments to parse
+/// \param [out] diags DiagnosticsEngine to report erros with
+static bool parseIntegerOverflowArgs(CompilerInvocation &invoc,
+                                     llvm::opt::ArgList &args,
+                                     clang::DiagnosticsEngine &diags) {
+  Fortran::common::LangOptions &opts = invoc.getLangOpts();
+
+  if (args.getLastArg(clang::driver::options::OPT_fwrapv))
+    opts.setSignedOverflowBehavior(Fortran::common::LangOptions::SOB_Defined);
+
+  return true;
+}
+
 /// Parses all floating point related arguments and populates the
 /// CompilerInvocation accordingly.
 /// Returns false if new errors are generated.
@@ -1249,6 +1267,18 @@ static bool parseLinkerOptionsArgs(CompilerInvocation &invoc,
   return true;
 }
 
+static bool parseLangOptionsArgs(CompilerInvocation &invoc,
+                                 llvm::opt::ArgList &args,
+                                 clang::DiagnosticsEngine &diags) {
+  bool success = true;
+
+  success &= parseIntegerOverflowArgs(invoc, args, diags);
+  success &= parseFloatingPointArgs(invoc, args, diags);
+  success &= parseVScaleArgs(invoc, args, diags);
+
+  return success;
+}
+
 bool CompilerInvocation::createFromArgs(
     CompilerInvocation &invoc, llvm::ArrayRef<const char *> commandLineArgs,
     clang::DiagnosticsEngine &diags, const char *argv0) {
@@ -1357,9 +1387,7 @@ bool CompilerInvocation::createFromArgs(
   invoc.frontendOpts.mlirArgs =
       args.getAllArgValues(clang::driver::options::OPT_mmlir);
 
-  success &= parseFloatingPointArgs(invoc, args, diags);
-
-  success &= parseVScaleArgs(invoc, args, diags);
+  success &= parseLangOptionsArgs(invoc, args, diags);
 
   success &= parseLinkerOptionsArgs(invoc, args, diags);
 
@@ -1571,6 +1599,8 @@ void CompilerInvocation::setLoweringOptions() {
   loweringOpts.setUnderscoring(codegenOpts.Underscoring);
 
   const Fortran::common::LangOptions &langOptions = getLangOpts();
+  loweringOpts.setIntegerWrapAround(langOptions.getSignedOverflowBehavior() ==
+                                    Fortran::common::LangOptions::SOB_Defined);
   Fortran::common::MathOptionsBase &mathOpts = loweringOpts.getMathOptions();
   // TODO: when LangOptions are finalized, we can represent
   //       the math related options using Fortran::commmon::MathOptionsBase,
diff --git a/flang/test/Driver/frontend-forwarding.f90 b/flang/test/Driver/frontend-forwarding.f90
index 35adb47b56861e..382c1aa5d350b7 100644
--- a/flang/test/Driver/frontend-forwarding.f90
+++ b/flang/test/Driver/frontend-forwarding.f90
@@ -14,6 +14,7 @@
 ! RUN:     -fno-signed-zeros \
 ! RUN:     -fassociative-math \
 ! RUN:     -freciprocal-math \
+! RUN:     -fno-strict-overflow \
 ! RUN:     -fomit-frame-pointer \
 ! RUN:     -fpass-plugin=Bye%pluginext \
 ! RUN:     -fversion-loops-for-stride \
@@ -63,4 +64,5 @@
 ! CHECK: "-Rpass=inline"
 ! CHECK: "-mframe-pointer=none"
 ! CHECK: "-mllvm" "-print-before-all"
+! CHECK: "-fwrapv"
 ! CHECK: "-save-temps=obj"
diff --git a/flang/test/Driver/integer-overflow.f90 b/flang/test/Driver/integer-overflow.f90
new file mode 100644
index 00000000000000..2cfb36527a30ad
--- /dev/null
+++ b/flang/test/Driver/integer-overflow.f90
@@ -0,0 +1,10 @@
+! Test for correct forwarding of integer overflow flags from the compiler driver
+! to the frontend driver
+
+! RUN: %flang -### -fno-strict-overflow %s 2>&1 | FileCheck %s --check-prefixes CHECK,INDUCED
+! RUN: %flang -### -fstrict-overflow %s 2>&1 | FileCheck %s
+! RUN: %flang -### -fno-wrapv %s 2>&1 | FileCheck %s
+! RUN: %flang -### -fno-wrapv -fno-strict-overflow %s 2>&1 | FileCheck %s
+
+! CHECK-NOT: "-fno-wrapv"
+! INDUCED: "-fwrapv"

@llvmbot
Copy link
Collaborator

llvmbot commented Oct 3, 2024

@llvm/pr-subscribers-flang-fir-hlfir

Author: Yusuke MINATO (yus3710-fj)

Changes

This patch introduces the options for integer overflow flags into Flang.
The behavior is similar to that of Clang.


Full diff: https://github.com/llvm/llvm-project/pull/110061.diff

8 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+7-4)
  • (modified) clang/lib/Driver/ToolChains/Flang.cpp (+11)
  • (modified) flang/include/flang/Common/LangOptions.def (+2)
  • (modified) flang/include/flang/Common/LangOptions.h (+8)
  • (modified) flang/include/flang/Lower/LoweringOptions.def (+2-3)
  • (modified) flang/lib/Frontend/CompilerInvocation.cpp (+33-3)
  • (modified) flang/test/Driver/frontend-forwarding.f90 (+2)
  • (added) flang/test/Driver/integer-overflow.f90 (+10)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 9d183ff2d69b3c..4610cd917f685d 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3451,7 +3451,8 @@ def fno_strict_aliasing : Flag<["-"], "fno-strict-aliasing">, Group<f_Group>,
 def fstruct_path_tbaa : Flag<["-"], "fstruct-path-tbaa">, Group<f_Group>;
 def fno_struct_path_tbaa : Flag<["-"], "fno-struct-path-tbaa">, Group<f_Group>;
 def fno_strict_enums : Flag<["-"], "fno-strict-enums">, Group<f_Group>;
-def fno_strict_overflow : Flag<["-"], "fno-strict-overflow">, Group<f_Group>;
+def fno_strict_overflow : Flag<["-"], "fno-strict-overflow">, Group<f_Group>,
+  Visibility<[ClangOption, FlangOption]>;
 def fno_pointer_tbaa : Flag<["-"], "fno-pointer-tbaa">, Group<f_Group>;
 def fno_temp_file : Flag<["-"], "fno-temp-file">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option, CLOption, DXCOption]>, HelpText<
@@ -3467,7 +3468,8 @@ def fno_verbose_asm : Flag<["-"], "fno-verbose-asm">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option]>,
   MarshallingInfoNegativeFlag<CodeGenOpts<"AsmVerbose">>;
 def fno_working_directory : Flag<["-"], "fno-working-directory">, Group<f_Group>;
-def fno_wrapv : Flag<["-"], "fno-wrapv">, Group<f_Group>;
+def fno_wrapv : Flag<["-"], "fno-wrapv">, Group<f_Group>,
+  Visibility<[ClangOption, FlangOption]>;
 def fobjc_arc : Flag<["-"], "fobjc-arc">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option]>,
   HelpText<"Synthesize retain and release calls for Objective-C pointers">;
@@ -3963,7 +3965,8 @@ defm strict_vtable_pointers : BoolFOption<"strict-vtable-pointers",
           "Enable optimizations based on the strict rules for"
             " overwriting polymorphic C++ objects">,
   NegFlag<SetFalse>>;
-def fstrict_overflow : Flag<["-"], "fstrict-overflow">, Group<f_Group>;
+def fstrict_overflow : Flag<["-"], "fstrict-overflow">, Group<f_Group>,
+  Visibility<[ClangOption, FlangOption]>;
 def fpointer_tbaa : Flag<["-"], "fpointer-tbaa">, Group<f_Group>;
 def fdriver_only : Flag<["-"], "fdriver-only">, Flags<[NoXarchOption]>,
   Visibility<[ClangOption, CLOption, DXCOption]>,
@@ -4232,7 +4235,7 @@ defm virtual_function_elimination : BoolFOption<"virtual-function-elimination",
   NegFlag<SetFalse>, BothFlags<[], [ClangOption, CLOption]>>;
 
 def fwrapv : Flag<["-"], "fwrapv">, Group<f_Group>,
-  Visibility<[ClangOption, CC1Option]>,
+  Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>,
   HelpText<"Treat signed integer overflow as two's complement">;
 def fwritable_strings : Flag<["-"], "fwritable-strings">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option]>,
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 98350690f8d20e..1865fd57a2d893 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -866,6 +866,17 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
     }
   }
 
+  // -fno-strict-overflow implies -fwrapv if it isn't disabled, but
+  // -fstrict-overflow won't turn off an explicitly enabled -fwrapv.
+  if (Arg *A = Args.getLastArg(options::OPT_fwrapv, options::OPT_fno_wrapv)) {
+    if (A->getOption().matches(options::OPT_fwrapv))
+      CmdArgs.push_back("-fwrapv");
+  } else if (Arg *A = Args.getLastArg(options::OPT_fstrict_overflow,
+                                      options::OPT_fno_strict_overflow)) {
+    if (A->getOption().matches(options::OPT_fno_strict_overflow))
+      CmdArgs.push_back("-fwrapv");
+  }
+
   assert((Output.isFilename() || Output.isNothing()) && "Invalid output.");
   if (Output.isFilename()) {
     CmdArgs.push_back("-o");
diff --git a/flang/include/flang/Common/LangOptions.def b/flang/include/flang/Common/LangOptions.def
index d3e1e972d1519f..1bfdba9cc2c1c7 100644
--- a/flang/include/flang/Common/LangOptions.def
+++ b/flang/include/flang/Common/LangOptions.def
@@ -20,6 +20,8 @@ LANGOPT(Name, Bits, Default)
 #endif
 
 ENUM_LANGOPT(FPContractMode, FPModeKind, 2, FPM_Fast) ///< FP Contract Mode (off/fast)
+/// signed integer overflow handling
+ENUM_LANGOPT(SignedOverflowBehavior, SignedOverflowBehaviorTy, 1, SOB_Undefined)
 
 /// Indicate a build without the standard GPU libraries.
 LANGOPT(NoGPULib  , 1, false)
diff --git a/flang/include/flang/Common/LangOptions.h b/flang/include/flang/Common/LangOptions.h
index 52a45047deb0e2..83f25cfbe26142 100644
--- a/flang/include/flang/Common/LangOptions.h
+++ b/flang/include/flang/Common/LangOptions.h
@@ -27,6 +27,14 @@ namespace Fortran::common {
 class LangOptionsBase {
 
 public:
+  enum SignedOverflowBehaviorTy {
+    // -fno-wrapv (default behavior in Flang)
+    SOB_Undefined,
+
+    // -fwrapv
+    SOB_Defined,
+  };
+
   enum FPModeKind {
     // Do not fuse FP ops
     FPM_Off,
diff --git a/flang/include/flang/Lower/LoweringOptions.def b/flang/include/flang/Lower/LoweringOptions.def
index d3f17c3f939c16..231de533fbd30a 100644
--- a/flang/include/flang/Lower/LoweringOptions.def
+++ b/flang/include/flang/Lower/LoweringOptions.def
@@ -35,9 +35,8 @@ ENUM_LOWERINGOPT(NoPPCNativeVecElemOrder, unsigned, 1, 0)
 ENUM_LOWERINGOPT(Underscoring, unsigned, 1, 1)
 
 /// If true, assume the behavior of integer overflow is defined
-/// (i.e. wraps around as two's complement). On by default.
-/// TODO: make the default off
-ENUM_LOWERINGOPT(IntegerWrapAround, unsigned, 1, 1)
+/// (i.e. wraps around as two's complement). Off by default.
+ENUM_LOWERINGOPT(IntegerWrapAround, unsigned, 1, 0)
 
 /// If true, add nsw flags to loop variable increments.
 /// Off by default.
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 2154b9ab2fbf47..c8160a4236b466 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -1109,6 +1109,24 @@ static bool parseOpenMPArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
   return diags.getNumErrors() == numErrorsBefore;
 }
 
+/// Parses signed integer overflow options and populates the
+/// CompilerInvocation accordingly.
+/// Returns false if new errors are generated.
+///
+/// \param [out] invoc Stores the processed arguments
+/// \param [in] args The compiler invocation arguments to parse
+/// \param [out] diags DiagnosticsEngine to report erros with
+static bool parseIntegerOverflowArgs(CompilerInvocation &invoc,
+                                     llvm::opt::ArgList &args,
+                                     clang::DiagnosticsEngine &diags) {
+  Fortran::common::LangOptions &opts = invoc.getLangOpts();
+
+  if (args.getLastArg(clang::driver::options::OPT_fwrapv))
+    opts.setSignedOverflowBehavior(Fortran::common::LangOptions::SOB_Defined);
+
+  return true;
+}
+
 /// Parses all floating point related arguments and populates the
 /// CompilerInvocation accordingly.
 /// Returns false if new errors are generated.
@@ -1249,6 +1267,18 @@ static bool parseLinkerOptionsArgs(CompilerInvocation &invoc,
   return true;
 }
 
+static bool parseLangOptionsArgs(CompilerInvocation &invoc,
+                                 llvm::opt::ArgList &args,
+                                 clang::DiagnosticsEngine &diags) {
+  bool success = true;
+
+  success &= parseIntegerOverflowArgs(invoc, args, diags);
+  success &= parseFloatingPointArgs(invoc, args, diags);
+  success &= parseVScaleArgs(invoc, args, diags);
+
+  return success;
+}
+
 bool CompilerInvocation::createFromArgs(
     CompilerInvocation &invoc, llvm::ArrayRef<const char *> commandLineArgs,
     clang::DiagnosticsEngine &diags, const char *argv0) {
@@ -1357,9 +1387,7 @@ bool CompilerInvocation::createFromArgs(
   invoc.frontendOpts.mlirArgs =
       args.getAllArgValues(clang::driver::options::OPT_mmlir);
 
-  success &= parseFloatingPointArgs(invoc, args, diags);
-
-  success &= parseVScaleArgs(invoc, args, diags);
+  success &= parseLangOptionsArgs(invoc, args, diags);
 
   success &= parseLinkerOptionsArgs(invoc, args, diags);
 
@@ -1571,6 +1599,8 @@ void CompilerInvocation::setLoweringOptions() {
   loweringOpts.setUnderscoring(codegenOpts.Underscoring);
 
   const Fortran::common::LangOptions &langOptions = getLangOpts();
+  loweringOpts.setIntegerWrapAround(langOptions.getSignedOverflowBehavior() ==
+                                    Fortran::common::LangOptions::SOB_Defined);
   Fortran::common::MathOptionsBase &mathOpts = loweringOpts.getMathOptions();
   // TODO: when LangOptions are finalized, we can represent
   //       the math related options using Fortran::commmon::MathOptionsBase,
diff --git a/flang/test/Driver/frontend-forwarding.f90 b/flang/test/Driver/frontend-forwarding.f90
index 35adb47b56861e..382c1aa5d350b7 100644
--- a/flang/test/Driver/frontend-forwarding.f90
+++ b/flang/test/Driver/frontend-forwarding.f90
@@ -14,6 +14,7 @@
 ! RUN:     -fno-signed-zeros \
 ! RUN:     -fassociative-math \
 ! RUN:     -freciprocal-math \
+! RUN:     -fno-strict-overflow \
 ! RUN:     -fomit-frame-pointer \
 ! RUN:     -fpass-plugin=Bye%pluginext \
 ! RUN:     -fversion-loops-for-stride \
@@ -63,4 +64,5 @@
 ! CHECK: "-Rpass=inline"
 ! CHECK: "-mframe-pointer=none"
 ! CHECK: "-mllvm" "-print-before-all"
+! CHECK: "-fwrapv"
 ! CHECK: "-save-temps=obj"
diff --git a/flang/test/Driver/integer-overflow.f90 b/flang/test/Driver/integer-overflow.f90
new file mode 100644
index 00000000000000..2cfb36527a30ad
--- /dev/null
+++ b/flang/test/Driver/integer-overflow.f90
@@ -0,0 +1,10 @@
+! Test for correct forwarding of integer overflow flags from the compiler driver
+! to the frontend driver
+
+! RUN: %flang -### -fno-strict-overflow %s 2>&1 | FileCheck %s --check-prefixes CHECK,INDUCED
+! RUN: %flang -### -fstrict-overflow %s 2>&1 | FileCheck %s
+! RUN: %flang -### -fno-wrapv %s 2>&1 | FileCheck %s
+! RUN: %flang -### -fno-wrapv -fno-strict-overflow %s 2>&1 | FileCheck %s
+
+! CHECK-NOT: "-fno-wrapv"
+! INDUCED: "-fwrapv"

Comment on lines 869 to 873
// -fno-strict-overflow implies -fwrapv if it isn't disabled, but
// -fstrict-overflow won't turn off an explicitly enabled -fwrapv.
if (Arg *A = Args.getLastArg(options::OPT_fwrapv, options::OPT_fno_wrapv)) {
if (A->getOption().matches(options::OPT_fwrapv))
CmdArgs.push_back("-fwrapv");
} else if (Arg *A = Args.getLastArg(options::OPT_fstrict_overflow,
options::OPT_fno_strict_overflow)) {
if (A->getOption().matches(options::OPT_fno_strict_overflow))
CmdArgs.push_back("-fwrapv");
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this code is exactly the same as it is in clang, it would be better if it is shared. In clang/lib/Driver/ToolChains/CommonArgs.cpp, see handleColorDiagnosticsArgs for a case of common argument handling functionality between clang and flang. This was done in #109210

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for showing me a better way.
I created a function renderIntegerOverflowOptions to handle common options between clang and flang. However, I'm not sure the name of the function is appropriate because there are other related options handled only by clang (e.g. -ftrapv).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the function could be named renderCommonIntegerOverflowOptions which suggests that it does not handle all overflow options?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds good. I fixed it. Thank you.

! Test for correct forwarding of integer overflow flags from the compiler driver
! to the frontend driver

! RUN: %flang -### -fno-strict-overflow %s 2>&1 | FileCheck %s --check-prefixes CHECK,INDUCED
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need both the CHECK and INDUCED here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you mentioned, the check for CHECK is redundant. I removed it.

if (A->getOption().matches(options::OPT_fno_strict_overflow))
CmdArgs.push_back("-fwrapv");
}
// handle -f[no-]wrapv and -f[no-]strict-overflow, which are used by both
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Handle ...

Capitalization and adding a trailing period.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for pointing it out. I fixed it.

@@ -27,6 +27,14 @@ namespace Fortran::common {
class LangOptionsBase {

public:
enum SignedOverflowBehaviorTy {
// -fno-wrapv (default behavior in Flang)
SOB_Undefined,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused? Not sure that this definition is useful

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used as the default value of a LangOption SignedOverflowBehavior. If the behavior is not defined, nsw could be added to some operations such as add.
FWIW, this implementation is simliar to that of Clang.

Copy link
Contributor

@tarunprabhu tarunprabhu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the changes, Minato san. LGTM.

@yus3710-fj
Copy link
Contributor Author

Gentle ping @MaskRay.

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, thanks!

Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clangDriver part LG.

@yus3710-fj
Copy link
Contributor Author

Thank you all for the review! I'll merge this after rebasing.

…ow in the frontend

This patch introduces the options for integer overflow flags into Flang.
@yus3710-fj
Copy link
Contributor Author

CI failed due to ClangdTests/25/79 (DynamicIndexIncludeInsertion in CodeCompleteTests.cpp?). It's weird because there is no functional change on clang (and clangd). In addition, I can't reproduce this on my machine.
I'll try re-running CI with an empty commit.

@yus3710-fj
Copy link
Contributor Author

CI failed again, but the test mentioned above was passed this time. So, I think I can merge this.

@yus3710-fj yus3710-fj merged commit 9698e57 into llvm:main Oct 18, 2024
6 of 8 checks passed
@yus3710-fj yus3710-fj deleted the options-for-overflow branch October 18, 2024 07:30
bricknerb pushed a commit to bricknerb/llvm-project that referenced this pull request Oct 21, 2024
…ow in the frontend (llvm#110061)

This patch introduces the options for integer overflow flags into Flang.
The behavior is similar to that of Clang.
EricWF pushed a commit to efcs/llvm-project that referenced this pull request Oct 22, 2024
…ow in the frontend (llvm#110061)

This patch introduces the options for integer overflow flags into Flang.
The behavior is similar to that of Clang.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category flang:driver flang:fir-hlfir flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Flang] TSVC s243: some nsw flags for address calculations will help vectorization
5 participants