Skip to content

[flang] Add nsw flag to do-variable increment with a new option #91579

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

Merged
merged 1 commit into from
May 16, 2024

Conversation

yus3710-fj
Copy link
Contributor

This patch adds nsw flag to the increment of do-variables when a new option is enabled.
NOTE 11.10 in the Fortran 2018 standard says they never overflow.

See also the discussion in #74709 and the following discourse post.
https://discourse.llvm.org/t/rfc-add-nsw-flags-to-arithmetic-integer-operations-using-the-option-fno-wrapv/77584/5

@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 May 9, 2024
@llvmbot
Copy link
Member

llvmbot commented May 9, 2024

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

@llvm/pr-subscribers-clang

Author: Yusuke MINATO (yus3710-fj)

Changes

This patch adds nsw flag to the increment of do-variables when a new option is enabled.
NOTE 11.10 in the Fortran 2018 standard says they never overflow.

See also the discussion in #74709 and the following discourse post.
https://discourse.llvm.org/t/rfc-add-nsw-flags-to-arithmetic-integer-operations-using-the-option-fno-wrapv/77584/5


Patch is 70.29 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/91579.diff

20 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+4)
  • (modified) clang/lib/Driver/ToolChains/Flang.cpp (+1)
  • (modified) flang/include/flang/Lower/LoweringOptions.def (+4)
  • (modified) flang/include/flang/Optimizer/Transforms/Passes.h (+3-1)
  • (modified) flang/include/flang/Optimizer/Transforms/Passes.td (+4-1)
  • (modified) flang/include/flang/Tools/CLOptions.inc (+9-4)
  • (modified) flang/include/flang/Tools/CrossToolHelpers.h (+1)
  • (modified) flang/lib/Frontend/CompilerInvocation.cpp (+6)
  • (modified) flang/lib/Frontend/FrontendActions.cpp (+3)
  • (modified) flang/lib/Lower/Bridge.cpp (+9-3)
  • (modified) flang/lib/Lower/IO.cpp (+7-2)
  • (modified) flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp (+34-10)
  • (modified) flang/test/Driver/frontend-forwarding.f90 (+2)
  • (modified) flang/test/Fir/loop01.fir (+211)
  • (modified) flang/test/Lower/array-substring.f90 (+40)
  • (modified) flang/test/Lower/do_loop.f90 (+42)
  • (modified) flang/test/Lower/do_loop_unstructured.f90 (+188-1)
  • (modified) flang/test/Lower/infinite_loop.f90 (+34)
  • (modified) flang/test/Lower/io-implied-do-fixes.f90 (+50-1)
  • (modified) flang/tools/bbc/bbc.cpp (+7)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 938d5358eeda6..a59da22fbdfc5 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6530,6 +6530,10 @@ def flang_deprecated_no_hlfir : Flag<["-"], "flang-deprecated-no-hlfir">,
   Flags<[HelpHidden]>, Visibility<[FlangOption, FC1Option]>,
   HelpText<"Do not use HLFIR lowering (deprecated)">;
 
+def flang_experimental_integer_overflow : Flag<["-"], "flang-experimental-integer-overflow">,
+  Flags<[HelpHidden]>, Visibility<[FlangOption, FC1Option]>,
+  HelpText<"Add nsw flag to internal operations such as do-variable increment (experimental)">;
+
 //===----------------------------------------------------------------------===//
 // FLangOption + CoreOption + NoXarchOption
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 8955b9fb653c2..677a598aa77c2 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -139,6 +139,7 @@ void Flang::addCodegenOptions(const ArgList &Args,
 
   Args.addAllArgs(CmdArgs, {options::OPT_flang_experimental_hlfir,
                             options::OPT_flang_deprecated_no_hlfir,
+                            options::OPT_flang_experimental_integer_overflow,
                             options::OPT_fno_ppc_native_vec_elem_order,
                             options::OPT_fppc_native_vec_elem_order});
 }
diff --git a/flang/include/flang/Lower/LoweringOptions.def b/flang/include/flang/Lower/LoweringOptions.def
index be080a4d29d73..839d2b46249b0 100644
--- a/flang/include/flang/Lower/LoweringOptions.def
+++ b/flang/include/flang/Lower/LoweringOptions.def
@@ -34,5 +34,9 @@ ENUM_LOWERINGOPT(NoPPCNativeVecElemOrder, unsigned, 1, 0)
 /// On by default.
 ENUM_LOWERINGOPT(Underscoring, unsigned, 1, 1)
 
+/// If true, add nsw flags to arithmetic operations for integer.
+/// Off by default.
+ENUM_LOWERINGOPT(NoSignedWrap, unsigned, 1, 0)
+
 #undef LOWERINGOPT
 #undef ENUM_LOWERINGOPT
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.h b/flang/include/flang/Optimizer/Transforms/Passes.h
index 470ed8a125ac4..496201a04e29c 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.h
+++ b/flang/include/flang/Optimizer/Transforms/Passes.h
@@ -54,6 +54,7 @@ namespace fir {
 std::unique_ptr<mlir::Pass> createAffineDemotionPass();
 std::unique_ptr<mlir::Pass>
 createArrayValueCopyPass(fir::ArrayValueCopyOptions options = {});
+std::unique_ptr<mlir::Pass> createCFGConversionPassWithNSW();
 std::unique_ptr<mlir::Pass> createExternalNameConversionPass();
 std::unique_ptr<mlir::Pass>
 createExternalNameConversionPass(bool appendUnderscore);
@@ -89,7 +90,8 @@ createFunctionAttrPass(FunctionAttrTypes &functionAttr, bool noInfsFPMath,
                        bool noSignedZerosFPMath, bool unsafeFPMath);
 
 void populateCfgConversionRewrites(mlir::RewritePatternSet &patterns,
-                                   bool forceLoopToExecuteOnce = false);
+                                   bool forceLoopToExecuteOnce = false,
+                                   bool setNSW = false);
 
 // declarative passes
 #define GEN_PASS_REGISTRATION
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.td b/flang/include/flang/Optimizer/Transforms/Passes.td
index 1eaaa32a508a0..ecbf8d5577b04 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.td
+++ b/flang/include/flang/Optimizer/Transforms/Passes.td
@@ -151,7 +151,10 @@ def CFGConversion : Pass<"cfg-conversion"> {
   let options = [
     Option<"forceLoopToExecuteOnce", "always-execute-loop-body", "bool",
            /*default=*/"false",
-           "force the body of a loop to execute at least once">
+           "force the body of a loop to execute at least once">,
+    Option<"setNSW", "set-nsw", "bool",
+           /*default=*/"false",
+           "set nsw on loop variable increment">
   ];
 }
 
diff --git a/flang/include/flang/Tools/CLOptions.inc b/flang/include/flang/Tools/CLOptions.inc
index 79a2a4f63cfcf..5ad7df714d348 100644
--- a/flang/include/flang/Tools/CLOptions.inc
+++ b/flang/include/flang/Tools/CLOptions.inc
@@ -148,9 +148,14 @@ static void addCanonicalizerPassWithoutRegionSimplification(
   pm.addPass(mlir::createCanonicalizerPass(config));
 }
 
-inline void addCfgConversionPass(mlir::PassManager &pm) {
-  addNestedPassToAllTopLevelOperationsConditionally(
-      pm, disableCfgConversion, fir::createCFGConversion);
+inline void addCfgConversionPass(
+    mlir::PassManager &pm, const MLIRToLLVMPassPipelineConfig &config) {
+  if (config.NoSignedWrap)
+    addNestedPassToAllTopLevelOperationsConditionally(
+        pm, disableCfgConversion, fir::createCFGConversionPassWithNSW);
+  else
+    addNestedPassToAllTopLevelOperationsConditionally(
+        pm, disableCfgConversion, fir::createCFGConversion);
 }
 
 inline void addAVC(
@@ -290,7 +295,7 @@ inline void createDefaultFIROptimizerPassPipeline(
     pm.addPass(fir::createAliasTagsPass());
 
   // convert control flow to CFG form
-  fir::addCfgConversionPass(pm);
+  fir::addCfgConversionPass(pm, pc);
   pm.addPass(mlir::createConvertSCFToCFPass());
 
   pm.addPass(mlir::createCanonicalizerPass(config));
diff --git a/flang/include/flang/Tools/CrossToolHelpers.h b/flang/include/flang/Tools/CrossToolHelpers.h
index f79520707714d..583daa30289d6 100644
--- a/flang/include/flang/Tools/CrossToolHelpers.h
+++ b/flang/include/flang/Tools/CrossToolHelpers.h
@@ -122,6 +122,7 @@ struct MLIRToLLVMPassPipelineConfig : public FlangEPCallBacks {
   bool NoSignedZerosFPMath =
       false; ///< Set no-signed-zeros-fp-math attribute for functions.
   bool UnsafeFPMath = false; ///< Set unsafe-fp-math attribute for functions.
+  bool NoSignedWrap = false; ///< Add nsw flag to numeric operations.
 };
 
 struct OffloadModuleOpts {
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index f1b7b53975398..3ed1cb10ae08c 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -1198,6 +1198,12 @@ bool CompilerInvocation::createFromArgs(
     invoc.loweringOpts.setNoPPCNativeVecElemOrder(true);
   }
 
+  // -flang-experimental-integer-overflow
+  if (args.hasArg(
+          clang::driver::options::OPT_flang_experimental_integer_overflow)) {
+    invoc.loweringOpts.setNoSignedWrap(true);
+  }
+
   // Preserve all the remark options requested, i.e. -Rpass, -Rpass-missed or
   // -Rpass-analysis. This will be used later when processing and outputting the
   // remarks generated by LLVM in ExecuteCompilerInvocation.cpp.
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 2f65ab6102f4d..deced43462607 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -809,6 +809,9 @@ void CodeGenAction::generateLLVMIR() {
     config.VScaleMax = vsr->second;
   }
 
+  if (ci.getInvocation().getLoweringOpts().getNoSignedWrap())
+    config.NoSignedWrap = true;
+
   // Create the pass pipeline
   fir::createMLIRToLLVMPassPipeline(pm, config, getCurrentFile());
   (void)mlir::applyPassManagerCLOptions(pm);
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index ae8679afc603f..a596d4933668b 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -1996,6 +1996,11 @@ class FirConverter : public Fortran::lower::AbstractConverter {
   void genFIRIncrementLoopEnd(IncrementLoopNestInfo &incrementLoopNestInfo) {
     assert(!incrementLoopNestInfo.empty() && "empty loop nest");
     mlir::Location loc = toLocation();
+    mlir::arith::IntegerOverflowFlags flags{};
+    if (getLoweringOptions().getNoSignedWrap())
+      flags = bitEnumSet(flags, mlir::arith::IntegerOverflowFlags::nsw);
+    auto iofAttr = mlir::arith::IntegerOverflowFlagsAttr::get(
+        builder->getContext(), flags);
     for (auto it = incrementLoopNestInfo.rbegin(),
               rend = incrementLoopNestInfo.rend();
          it != rend; ++it) {
@@ -2010,7 +2015,8 @@ class FirConverter : public Fortran::lower::AbstractConverter {
         builder->setInsertionPointToEnd(info.doLoop.getBody());
         llvm::SmallVector<mlir::Value, 2> results;
         results.push_back(builder->create<mlir::arith::AddIOp>(
-            loc, info.doLoop.getInductionVar(), info.doLoop.getStep()));
+            loc, info.doLoop.getInductionVar(), info.doLoop.getStep(),
+            iofAttr));
         // Step loopVariable to help optimizations such as vectorization.
         // Induction variable elimination will clean up as necessary.
         mlir::Value step = builder->createConvert(
@@ -2018,7 +2024,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
         mlir::Value loopVar =
             builder->create<fir::LoadOp>(loc, info.loopVariable);
         results.push_back(
-            builder->create<mlir::arith::AddIOp>(loc, loopVar, step));
+            builder->create<mlir::arith::AddIOp>(loc, loopVar, step, iofAttr));
         builder->create<fir::ResultOp>(loc, results);
         builder->setInsertionPointAfter(info.doLoop);
         // The loop control variable may be used after the loop.
@@ -2043,7 +2049,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
       if (info.hasRealControl)
         value = builder->create<mlir::arith::AddFOp>(loc, value, step);
       else
-        value = builder->create<mlir::arith::AddIOp>(loc, value, step);
+        value = builder->create<mlir::arith::AddIOp>(loc, value, step, iofAttr);
       builder->create<fir::StoreOp>(loc, value, info.loopVariable);
 
       genBranch(info.headerBlock);
diff --git a/flang/lib/Lower/IO.cpp b/flang/lib/Lower/IO.cpp
index ed0afad9197df..cc0347a4771eb 100644
--- a/flang/lib/Lower/IO.cpp
+++ b/flang/lib/Lower/IO.cpp
@@ -928,6 +928,11 @@ static void genIoLoop(Fortran::lower::AbstractConverter &converter,
   Fortran::lower::StatementContext stmtCtx;
   fir::FirOpBuilder &builder = converter.getFirOpBuilder();
   mlir::Location loc = converter.getCurrentLocation();
+  mlir::arith::IntegerOverflowFlags flags{};
+  if (converter.getLoweringOptions().getNoSignedWrap())
+    flags = bitEnumSet(flags, mlir::arith::IntegerOverflowFlags::nsw);
+  auto iofAttr =
+      mlir::arith::IntegerOverflowFlagsAttr::get(builder.getContext(), flags);
   makeNextConditionalOn(builder, loc, checkResult, ok, inLoop);
   const auto &itemList = std::get<0>(ioImpliedDo.t);
   const auto &control = std::get<1>(ioImpliedDo.t);
@@ -965,7 +970,7 @@ static void genIoLoop(Fortran::lower::AbstractConverter &converter,
     genItemList(ioImpliedDo);
     builder.setInsertionPointToEnd(doLoopOp.getBody());
     mlir::Value result = builder.create<mlir::arith::AddIOp>(
-        loc, doLoopOp.getInductionVar(), doLoopOp.getStep());
+        loc, doLoopOp.getInductionVar(), doLoopOp.getStep(), iofAttr);
     builder.create<fir::ResultOp>(loc, result);
     builder.setInsertionPointAfter(doLoopOp);
     // The loop control variable may be used after the loop.
@@ -1007,7 +1012,7 @@ static void genIoLoop(Fortran::lower::AbstractConverter &converter,
   mlir::OpResult iterateResult = builder.getBlock()->back().getResult(0);
   mlir::Value inductionResult0 = iterWhileOp.getInductionVar();
   auto inductionResult1 = builder.create<mlir::arith::AddIOp>(
-      loc, inductionResult0, iterWhileOp.getStep());
+      loc, inductionResult0, iterWhileOp.getStep(), iofAttr);
   auto inductionResult = builder.create<mlir::arith::SelectOp>(
       loc, iterateResult, inductionResult1, inductionResult0);
   llvm::SmallVector<mlir::Value> results = {inductionResult, iterateResult};
diff --git a/flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp b/flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp
index a62f6cde0e09b..a233e7fbdcd1e 100644
--- a/flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp
+++ b/flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp
@@ -43,14 +43,19 @@ class CfgLoopConv : public mlir::OpRewritePattern<fir::DoLoopOp> {
 public:
   using OpRewritePattern::OpRewritePattern;
 
-  CfgLoopConv(mlir::MLIRContext *ctx, bool forceLoopToExecuteOnce)
+  CfgLoopConv(mlir::MLIRContext *ctx, bool forceLoopToExecuteOnce, bool setNSW)
       : mlir::OpRewritePattern<fir::DoLoopOp>(ctx),
-        forceLoopToExecuteOnce(forceLoopToExecuteOnce) {}
+        forceLoopToExecuteOnce(forceLoopToExecuteOnce), setNSW(setNSW) {}
 
   mlir::LogicalResult
   matchAndRewrite(DoLoopOp loop,
                   mlir::PatternRewriter &rewriter) const override {
     auto loc = loop.getLoc();
+    mlir::arith::IntegerOverflowFlags flags{};
+    if (setNSW)
+      flags = bitEnumSet(flags, mlir::arith::IntegerOverflowFlags::nsw);
+    auto iofAttr = mlir::arith::IntegerOverflowFlagsAttr::get(
+        rewriter.getContext(), flags);
 
     // Create the start and end blocks that will wrap the DoLoopOp with an
     // initalizer and an end point
@@ -104,7 +109,7 @@ class CfgLoopConv : public mlir::OpRewritePattern<fir::DoLoopOp> {
     rewriter.setInsertionPointToEnd(lastBlock);
     auto iv = conditionalBlock->getArgument(0);
     mlir::Value steppedIndex =
-        rewriter.create<mlir::arith::AddIOp>(loc, iv, step);
+        rewriter.create<mlir::arith::AddIOp>(loc, iv, step, iofAttr);
     assert(steppedIndex && "must be a Value");
     auto lastArg = conditionalBlock->getNumArguments() - 1;
     auto itersLeft = conditionalBlock->getArgument(lastArg);
@@ -142,6 +147,7 @@ class CfgLoopConv : public mlir::OpRewritePattern<fir::DoLoopOp> {
 
 private:
   bool forceLoopToExecuteOnce;
+  bool setNSW;
 };
 
 /// Convert `fir.if` to control-flow
@@ -149,7 +155,7 @@ class CfgIfConv : public mlir::OpRewritePattern<fir::IfOp> {
 public:
   using OpRewritePattern::OpRewritePattern;
 
-  CfgIfConv(mlir::MLIRContext *ctx, bool forceLoopToExecuteOnce)
+  CfgIfConv(mlir::MLIRContext *ctx, bool forceLoopToExecuteOnce, bool setNSW)
       : mlir::OpRewritePattern<fir::IfOp>(ctx) {}
 
   mlir::LogicalResult
@@ -214,13 +220,19 @@ class CfgIterWhileConv : public mlir::OpRewritePattern<fir::IterWhileOp> {
 public:
   using OpRewritePattern::OpRewritePattern;
 
-  CfgIterWhileConv(mlir::MLIRContext *ctx, bool forceLoopToExecuteOnce)
-      : mlir::OpRewritePattern<fir::IterWhileOp>(ctx) {}
+  CfgIterWhileConv(mlir::MLIRContext *ctx, bool forceLoopToExecuteOnce,
+                   bool setNSW)
+      : mlir::OpRewritePattern<fir::IterWhileOp>(ctx), setNSW(setNSW) {}
 
   mlir::LogicalResult
   matchAndRewrite(fir::IterWhileOp whileOp,
                   mlir::PatternRewriter &rewriter) const override {
     auto loc = whileOp.getLoc();
+    mlir::arith::IntegerOverflowFlags flags{};
+    if (setNSW)
+      flags = bitEnumSet(flags, mlir::arith::IntegerOverflowFlags::nsw);
+    auto iofAttr = mlir::arith::IntegerOverflowFlagsAttr::get(
+        rewriter.getContext(), flags);
 
     // Start by splitting the block containing the 'fir.do_loop' into two parts.
     // The part before will get the init code, the part after will be the end
@@ -248,7 +260,8 @@ class CfgIterWhileConv : public mlir::OpRewritePattern<fir::IterWhileOp> {
     auto *terminator = lastBodyBlock->getTerminator();
     rewriter.setInsertionPointToEnd(lastBodyBlock);
     auto step = whileOp.getStep();
-    mlir::Value stepped = rewriter.create<mlir::arith::AddIOp>(loc, iv, step);
+    mlir::Value stepped =
+        rewriter.create<mlir::arith::AddIOp>(loc, iv, step, iofAttr);
     assert(stepped && "must be a Value");
 
     llvm::SmallVector<mlir::Value> loopCarried;
@@ -305,6 +318,9 @@ class CfgIterWhileConv : public mlir::OpRewritePattern<fir::IterWhileOp> {
     rewriter.replaceOp(whileOp, args);
     return success();
   }
+
+private:
+  bool setNSW;
 };
 
 /// Convert FIR structured control flow ops to CFG ops.
@@ -312,10 +328,13 @@ class CfgConversion : public fir::impl::CFGConversionBase<CfgConversion> {
 public:
   using CFGConversionBase<CfgConversion>::CFGConversionBase;
 
+  CfgConversion(bool setNSW) { this->setNSW = setNSW; }
+
   void runOnOperation() override {
     auto *context = &this->getContext();
     mlir::RewritePatternSet patterns(context);
-    fir::populateCfgConversionRewrites(patterns, this->forceLoopToExecuteOnce);
+    fir::populateCfgConversionRewrites(patterns, this->forceLoopToExecuteOnce,
+                                       this->setNSW);
     mlir::ConversionTarget target(*context);
     target.addLegalDialect<mlir::affine::AffineDialect,
                            mlir::cf::ControlFlowDialect, FIROpsDialect,
@@ -337,7 +356,12 @@ class CfgConversion : public fir::impl::CFGConversionBase<CfgConversion> {
 
 /// Expose conversion rewriters to other passes
 void fir::populateCfgConversionRewrites(mlir::RewritePatternSet &patterns,
-                                        bool forceLoopToExecuteOnce) {
+                                        bool forceLoopToExecuteOnce,
+                                        bool setNSW) {
   patterns.insert<CfgLoopConv, CfgIfConv, CfgIterWhileConv>(
-      patterns.getContext(), forceLoopToExecuteOnce);
+      patterns.getContext(), forceLoopToExecuteOnce, setNSW);
+}
+
+std::unique_ptr<mlir::Pass> fir::createCFGConversionPassWithNSW() {
+  return std::make_unique<CfgConversion>(true);
 }
diff --git a/flang/test/Driver/frontend-forwarding.f90 b/flang/test/Driver/frontend-forwarding.f90
index eac9773ce25c7..35adb47b56861 100644
--- a/flang/test/Driver/frontend-forwarding.f90
+++ b/flang/test/Driver/frontend-forwarding.f90
@@ -19,6 +19,7 @@
 ! RUN:     -fversion-loops-for-stride \
 ! RUN:     -flang-experimental-hlfir \
 ! RUN:     -flang-deprecated-no-hlfir \
+! RUN:     -flang-experimental-integer-overflow \
 ! RUN:     -fno-ppc-native-vector-element-order \
 ! RUN:     -fppc-native-vector-element-order \
 ! RUN:     -mllvm -print-before-all \
@@ -50,6 +51,7 @@
 ! CHECK: "-fversion-loops-for-stride"
 ! CHECK: "-flang-experimental-hlfir"
 ! CHECK: "-flang-deprecated-no-hlfir"
+! CHECK: "-flang-experimental-integer-overflow"
 ! CHECK: "-fno-ppc-native-vector-element-order"
 ! CHECK: "-fppc-native-vector-element-order"
 ! CHECK: "-Rpass"
diff --git a/flang/test/Fir/loop01.fir b/flang/test/Fir/loop01.fir
index 72ca1c3989e45..c1cbb522c378c 100644
--- a/flang/test/Fir/loop01.fir
+++ b/flang/test/Fir/loop01.fir
@@ -1,4 +1,5 @@
 // RUN: fir-opt --split-input-file --cfg-conversion %s | FileCheck %s
+// RUN: fir-opt --split-input-file --cfg-conversion="set-nsw=true" %s | FileCheck %s --check-prefix=NSW
 
 func.func @x(%lb : index, %ub : index, %step : index, %b : i1, %addr : !fir.ref<index>) {
   fir.do_loop %iv = %lb to %ub step %step unordered {
@@ -43,6 +44,34 @@ func.func private @f2() -> i1
 // CHECK:     }
 // CHECK:     func private @f2() -> i1
 
+// NSW:     func @x(%[[VAL_0:.*]]: index, %[[VAL_1:.*]]: index, %[[VAL_2:.*]]: index, %[[VAL_3:.*]]: i1, %[[VAL_4:.*]]: !fir.ref<index>) {
+// NSW:       %[[VAL_5:.*]] = arith.subi %[[VAL_1]], %[[VAL_0]] : index
+// NSW:       %[[VAL_6:.*]] = arith.addi %[[VAL_5]], %[[VAL_2]] : index
+// NSW:       %[[VAL_7:.*]] = arith.divsi %[[VAL_6]], %[[VAL_2]] : index
+// NSW:       br ^bb1(%[[VAL_0]], %[[VAL_7]] : index, index)
+// NSW:     ^bb1(%[[VAL_8:.*]]: index, %[[VAL_9:.*]]: index):
+// NSW:       %[[VAL_10:.*]] = arith.constant 0 : index
+// NSW:       %[[VAL_11:.*]] = arith.cmpi sgt, %[[VAL_9]], %[[VAL_10]] : index
+// NSW:       cond_br %[[VAL_11]], ^bb2, ^bb6
+// NSW:     ^bb2:
+// NSW:       cond_br %[[VAL_3]], ^bb3, ^bb4
+// NSW:     ^bb3:
+// NSW:       fir.store %[[VAL_8]] to %[[VAL_4]] : !fir.ref<index>
+// NSW:       br ^bb5
+// NSW:     ^bb4:
+// NSW:       %[[VAL_12:.*]] = arith.constant 0 : index
+// NSW:       fir.store %[[VAL_12]] to %[[VAL_4]] : !fir.ref<index>
+// NSW:       br ^bb5
+// NSW:     ^bb5:
+// NSW:       %[[VAL_13:.*]] = arith.addi %[[VAL_8]], %[[VAL_2]] overflow<nsw> : index
+// NSW:       %[[VAL_14:.*]] = arith.constant 1 : index
+// NSW:       %[[VAL_15:.*]] = arith.subi %[[VAL_9]], %[[VAL_14]] : index
+// NSW:       br ^bb1(%[[VAL_13]], %[[VAL_15]] : index, index)
+// NSW:     ^bb6:
+// NSW:       return
+// NSW:     }
+// NSW:     func private @f2() -> i1
+
 // -----
 
 func.func @x2(%lo : index, %up : index, %ok : i1) {
@@ -79,6 +108,29 @@ func.func private @f3(i16)
 // CHECK:...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented May 9, 2024

@llvm/pr-subscribers-clang-driver

Author: Yusuke MINATO (yus3710-fj)

Changes

This patch adds nsw flag to the increment of do-variables when a new option is enabled.
NOTE 11.10 in the Fortran 2018 standard says they never overflow.

See also the discussion in #74709 and the following discourse post.
https://discourse.llvm.org/t/rfc-add-nsw-flags-to-arithmetic-integer-operations-using-the-option-fno-wrapv/77584/5


Patch is 70.29 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/91579.diff

20 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+4)
  • (modified) clang/lib/Driver/ToolChains/Flang.cpp (+1)
  • (modified) flang/include/flang/Lower/LoweringOptions.def (+4)
  • (modified) flang/include/flang/Optimizer/Transforms/Passes.h (+3-1)
  • (modified) flang/include/flang/Optimizer/Transforms/Passes.td (+4-1)
  • (modified) flang/include/flang/Tools/CLOptions.inc (+9-4)
  • (modified) flang/include/flang/Tools/CrossToolHelpers.h (+1)
  • (modified) flang/lib/Frontend/CompilerInvocation.cpp (+6)
  • (modified) flang/lib/Frontend/FrontendActions.cpp (+3)
  • (modified) flang/lib/Lower/Bridge.cpp (+9-3)
  • (modified) flang/lib/Lower/IO.cpp (+7-2)
  • (modified) flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp (+34-10)
  • (modified) flang/test/Driver/frontend-forwarding.f90 (+2)
  • (modified) flang/test/Fir/loop01.fir (+211)
  • (modified) flang/test/Lower/array-substring.f90 (+40)
  • (modified) flang/test/Lower/do_loop.f90 (+42)
  • (modified) flang/test/Lower/do_loop_unstructured.f90 (+188-1)
  • (modified) flang/test/Lower/infinite_loop.f90 (+34)
  • (modified) flang/test/Lower/io-implied-do-fixes.f90 (+50-1)
  • (modified) flang/tools/bbc/bbc.cpp (+7)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 938d5358eeda6..a59da22fbdfc5 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6530,6 +6530,10 @@ def flang_deprecated_no_hlfir : Flag<["-"], "flang-deprecated-no-hlfir">,
   Flags<[HelpHidden]>, Visibility<[FlangOption, FC1Option]>,
   HelpText<"Do not use HLFIR lowering (deprecated)">;
 
+def flang_experimental_integer_overflow : Flag<["-"], "flang-experimental-integer-overflow">,
+  Flags<[HelpHidden]>, Visibility<[FlangOption, FC1Option]>,
+  HelpText<"Add nsw flag to internal operations such as do-variable increment (experimental)">;
+
 //===----------------------------------------------------------------------===//
 // FLangOption + CoreOption + NoXarchOption
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 8955b9fb653c2..677a598aa77c2 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -139,6 +139,7 @@ void Flang::addCodegenOptions(const ArgList &Args,
 
   Args.addAllArgs(CmdArgs, {options::OPT_flang_experimental_hlfir,
                             options::OPT_flang_deprecated_no_hlfir,
+                            options::OPT_flang_experimental_integer_overflow,
                             options::OPT_fno_ppc_native_vec_elem_order,
                             options::OPT_fppc_native_vec_elem_order});
 }
diff --git a/flang/include/flang/Lower/LoweringOptions.def b/flang/include/flang/Lower/LoweringOptions.def
index be080a4d29d73..839d2b46249b0 100644
--- a/flang/include/flang/Lower/LoweringOptions.def
+++ b/flang/include/flang/Lower/LoweringOptions.def
@@ -34,5 +34,9 @@ ENUM_LOWERINGOPT(NoPPCNativeVecElemOrder, unsigned, 1, 0)
 /// On by default.
 ENUM_LOWERINGOPT(Underscoring, unsigned, 1, 1)
 
+/// If true, add nsw flags to arithmetic operations for integer.
+/// Off by default.
+ENUM_LOWERINGOPT(NoSignedWrap, unsigned, 1, 0)
+
 #undef LOWERINGOPT
 #undef ENUM_LOWERINGOPT
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.h b/flang/include/flang/Optimizer/Transforms/Passes.h
index 470ed8a125ac4..496201a04e29c 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.h
+++ b/flang/include/flang/Optimizer/Transforms/Passes.h
@@ -54,6 +54,7 @@ namespace fir {
 std::unique_ptr<mlir::Pass> createAffineDemotionPass();
 std::unique_ptr<mlir::Pass>
 createArrayValueCopyPass(fir::ArrayValueCopyOptions options = {});
+std::unique_ptr<mlir::Pass> createCFGConversionPassWithNSW();
 std::unique_ptr<mlir::Pass> createExternalNameConversionPass();
 std::unique_ptr<mlir::Pass>
 createExternalNameConversionPass(bool appendUnderscore);
@@ -89,7 +90,8 @@ createFunctionAttrPass(FunctionAttrTypes &functionAttr, bool noInfsFPMath,
                        bool noSignedZerosFPMath, bool unsafeFPMath);
 
 void populateCfgConversionRewrites(mlir::RewritePatternSet &patterns,
-                                   bool forceLoopToExecuteOnce = false);
+                                   bool forceLoopToExecuteOnce = false,
+                                   bool setNSW = false);
 
 // declarative passes
 #define GEN_PASS_REGISTRATION
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.td b/flang/include/flang/Optimizer/Transforms/Passes.td
index 1eaaa32a508a0..ecbf8d5577b04 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.td
+++ b/flang/include/flang/Optimizer/Transforms/Passes.td
@@ -151,7 +151,10 @@ def CFGConversion : Pass<"cfg-conversion"> {
   let options = [
     Option<"forceLoopToExecuteOnce", "always-execute-loop-body", "bool",
            /*default=*/"false",
-           "force the body of a loop to execute at least once">
+           "force the body of a loop to execute at least once">,
+    Option<"setNSW", "set-nsw", "bool",
+           /*default=*/"false",
+           "set nsw on loop variable increment">
   ];
 }
 
diff --git a/flang/include/flang/Tools/CLOptions.inc b/flang/include/flang/Tools/CLOptions.inc
index 79a2a4f63cfcf..5ad7df714d348 100644
--- a/flang/include/flang/Tools/CLOptions.inc
+++ b/flang/include/flang/Tools/CLOptions.inc
@@ -148,9 +148,14 @@ static void addCanonicalizerPassWithoutRegionSimplification(
   pm.addPass(mlir::createCanonicalizerPass(config));
 }
 
-inline void addCfgConversionPass(mlir::PassManager &pm) {
-  addNestedPassToAllTopLevelOperationsConditionally(
-      pm, disableCfgConversion, fir::createCFGConversion);
+inline void addCfgConversionPass(
+    mlir::PassManager &pm, const MLIRToLLVMPassPipelineConfig &config) {
+  if (config.NoSignedWrap)
+    addNestedPassToAllTopLevelOperationsConditionally(
+        pm, disableCfgConversion, fir::createCFGConversionPassWithNSW);
+  else
+    addNestedPassToAllTopLevelOperationsConditionally(
+        pm, disableCfgConversion, fir::createCFGConversion);
 }
 
 inline void addAVC(
@@ -290,7 +295,7 @@ inline void createDefaultFIROptimizerPassPipeline(
     pm.addPass(fir::createAliasTagsPass());
 
   // convert control flow to CFG form
-  fir::addCfgConversionPass(pm);
+  fir::addCfgConversionPass(pm, pc);
   pm.addPass(mlir::createConvertSCFToCFPass());
 
   pm.addPass(mlir::createCanonicalizerPass(config));
diff --git a/flang/include/flang/Tools/CrossToolHelpers.h b/flang/include/flang/Tools/CrossToolHelpers.h
index f79520707714d..583daa30289d6 100644
--- a/flang/include/flang/Tools/CrossToolHelpers.h
+++ b/flang/include/flang/Tools/CrossToolHelpers.h
@@ -122,6 +122,7 @@ struct MLIRToLLVMPassPipelineConfig : public FlangEPCallBacks {
   bool NoSignedZerosFPMath =
       false; ///< Set no-signed-zeros-fp-math attribute for functions.
   bool UnsafeFPMath = false; ///< Set unsafe-fp-math attribute for functions.
+  bool NoSignedWrap = false; ///< Add nsw flag to numeric operations.
 };
 
 struct OffloadModuleOpts {
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index f1b7b53975398..3ed1cb10ae08c 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -1198,6 +1198,12 @@ bool CompilerInvocation::createFromArgs(
     invoc.loweringOpts.setNoPPCNativeVecElemOrder(true);
   }
 
+  // -flang-experimental-integer-overflow
+  if (args.hasArg(
+          clang::driver::options::OPT_flang_experimental_integer_overflow)) {
+    invoc.loweringOpts.setNoSignedWrap(true);
+  }
+
   // Preserve all the remark options requested, i.e. -Rpass, -Rpass-missed or
   // -Rpass-analysis. This will be used later when processing and outputting the
   // remarks generated by LLVM in ExecuteCompilerInvocation.cpp.
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 2f65ab6102f4d..deced43462607 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -809,6 +809,9 @@ void CodeGenAction::generateLLVMIR() {
     config.VScaleMax = vsr->second;
   }
 
+  if (ci.getInvocation().getLoweringOpts().getNoSignedWrap())
+    config.NoSignedWrap = true;
+
   // Create the pass pipeline
   fir::createMLIRToLLVMPassPipeline(pm, config, getCurrentFile());
   (void)mlir::applyPassManagerCLOptions(pm);
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index ae8679afc603f..a596d4933668b 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -1996,6 +1996,11 @@ class FirConverter : public Fortran::lower::AbstractConverter {
   void genFIRIncrementLoopEnd(IncrementLoopNestInfo &incrementLoopNestInfo) {
     assert(!incrementLoopNestInfo.empty() && "empty loop nest");
     mlir::Location loc = toLocation();
+    mlir::arith::IntegerOverflowFlags flags{};
+    if (getLoweringOptions().getNoSignedWrap())
+      flags = bitEnumSet(flags, mlir::arith::IntegerOverflowFlags::nsw);
+    auto iofAttr = mlir::arith::IntegerOverflowFlagsAttr::get(
+        builder->getContext(), flags);
     for (auto it = incrementLoopNestInfo.rbegin(),
               rend = incrementLoopNestInfo.rend();
          it != rend; ++it) {
@@ -2010,7 +2015,8 @@ class FirConverter : public Fortran::lower::AbstractConverter {
         builder->setInsertionPointToEnd(info.doLoop.getBody());
         llvm::SmallVector<mlir::Value, 2> results;
         results.push_back(builder->create<mlir::arith::AddIOp>(
-            loc, info.doLoop.getInductionVar(), info.doLoop.getStep()));
+            loc, info.doLoop.getInductionVar(), info.doLoop.getStep(),
+            iofAttr));
         // Step loopVariable to help optimizations such as vectorization.
         // Induction variable elimination will clean up as necessary.
         mlir::Value step = builder->createConvert(
@@ -2018,7 +2024,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
         mlir::Value loopVar =
             builder->create<fir::LoadOp>(loc, info.loopVariable);
         results.push_back(
-            builder->create<mlir::arith::AddIOp>(loc, loopVar, step));
+            builder->create<mlir::arith::AddIOp>(loc, loopVar, step, iofAttr));
         builder->create<fir::ResultOp>(loc, results);
         builder->setInsertionPointAfter(info.doLoop);
         // The loop control variable may be used after the loop.
@@ -2043,7 +2049,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
       if (info.hasRealControl)
         value = builder->create<mlir::arith::AddFOp>(loc, value, step);
       else
-        value = builder->create<mlir::arith::AddIOp>(loc, value, step);
+        value = builder->create<mlir::arith::AddIOp>(loc, value, step, iofAttr);
       builder->create<fir::StoreOp>(loc, value, info.loopVariable);
 
       genBranch(info.headerBlock);
diff --git a/flang/lib/Lower/IO.cpp b/flang/lib/Lower/IO.cpp
index ed0afad9197df..cc0347a4771eb 100644
--- a/flang/lib/Lower/IO.cpp
+++ b/flang/lib/Lower/IO.cpp
@@ -928,6 +928,11 @@ static void genIoLoop(Fortran::lower::AbstractConverter &converter,
   Fortran::lower::StatementContext stmtCtx;
   fir::FirOpBuilder &builder = converter.getFirOpBuilder();
   mlir::Location loc = converter.getCurrentLocation();
+  mlir::arith::IntegerOverflowFlags flags{};
+  if (converter.getLoweringOptions().getNoSignedWrap())
+    flags = bitEnumSet(flags, mlir::arith::IntegerOverflowFlags::nsw);
+  auto iofAttr =
+      mlir::arith::IntegerOverflowFlagsAttr::get(builder.getContext(), flags);
   makeNextConditionalOn(builder, loc, checkResult, ok, inLoop);
   const auto &itemList = std::get<0>(ioImpliedDo.t);
   const auto &control = std::get<1>(ioImpliedDo.t);
@@ -965,7 +970,7 @@ static void genIoLoop(Fortran::lower::AbstractConverter &converter,
     genItemList(ioImpliedDo);
     builder.setInsertionPointToEnd(doLoopOp.getBody());
     mlir::Value result = builder.create<mlir::arith::AddIOp>(
-        loc, doLoopOp.getInductionVar(), doLoopOp.getStep());
+        loc, doLoopOp.getInductionVar(), doLoopOp.getStep(), iofAttr);
     builder.create<fir::ResultOp>(loc, result);
     builder.setInsertionPointAfter(doLoopOp);
     // The loop control variable may be used after the loop.
@@ -1007,7 +1012,7 @@ static void genIoLoop(Fortran::lower::AbstractConverter &converter,
   mlir::OpResult iterateResult = builder.getBlock()->back().getResult(0);
   mlir::Value inductionResult0 = iterWhileOp.getInductionVar();
   auto inductionResult1 = builder.create<mlir::arith::AddIOp>(
-      loc, inductionResult0, iterWhileOp.getStep());
+      loc, inductionResult0, iterWhileOp.getStep(), iofAttr);
   auto inductionResult = builder.create<mlir::arith::SelectOp>(
       loc, iterateResult, inductionResult1, inductionResult0);
   llvm::SmallVector<mlir::Value> results = {inductionResult, iterateResult};
diff --git a/flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp b/flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp
index a62f6cde0e09b..a233e7fbdcd1e 100644
--- a/flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp
+++ b/flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp
@@ -43,14 +43,19 @@ class CfgLoopConv : public mlir::OpRewritePattern<fir::DoLoopOp> {
 public:
   using OpRewritePattern::OpRewritePattern;
 
-  CfgLoopConv(mlir::MLIRContext *ctx, bool forceLoopToExecuteOnce)
+  CfgLoopConv(mlir::MLIRContext *ctx, bool forceLoopToExecuteOnce, bool setNSW)
       : mlir::OpRewritePattern<fir::DoLoopOp>(ctx),
-        forceLoopToExecuteOnce(forceLoopToExecuteOnce) {}
+        forceLoopToExecuteOnce(forceLoopToExecuteOnce), setNSW(setNSW) {}
 
   mlir::LogicalResult
   matchAndRewrite(DoLoopOp loop,
                   mlir::PatternRewriter &rewriter) const override {
     auto loc = loop.getLoc();
+    mlir::arith::IntegerOverflowFlags flags{};
+    if (setNSW)
+      flags = bitEnumSet(flags, mlir::arith::IntegerOverflowFlags::nsw);
+    auto iofAttr = mlir::arith::IntegerOverflowFlagsAttr::get(
+        rewriter.getContext(), flags);
 
     // Create the start and end blocks that will wrap the DoLoopOp with an
     // initalizer and an end point
@@ -104,7 +109,7 @@ class CfgLoopConv : public mlir::OpRewritePattern<fir::DoLoopOp> {
     rewriter.setInsertionPointToEnd(lastBlock);
     auto iv = conditionalBlock->getArgument(0);
     mlir::Value steppedIndex =
-        rewriter.create<mlir::arith::AddIOp>(loc, iv, step);
+        rewriter.create<mlir::arith::AddIOp>(loc, iv, step, iofAttr);
     assert(steppedIndex && "must be a Value");
     auto lastArg = conditionalBlock->getNumArguments() - 1;
     auto itersLeft = conditionalBlock->getArgument(lastArg);
@@ -142,6 +147,7 @@ class CfgLoopConv : public mlir::OpRewritePattern<fir::DoLoopOp> {
 
 private:
   bool forceLoopToExecuteOnce;
+  bool setNSW;
 };
 
 /// Convert `fir.if` to control-flow
@@ -149,7 +155,7 @@ class CfgIfConv : public mlir::OpRewritePattern<fir::IfOp> {
 public:
   using OpRewritePattern::OpRewritePattern;
 
-  CfgIfConv(mlir::MLIRContext *ctx, bool forceLoopToExecuteOnce)
+  CfgIfConv(mlir::MLIRContext *ctx, bool forceLoopToExecuteOnce, bool setNSW)
       : mlir::OpRewritePattern<fir::IfOp>(ctx) {}
 
   mlir::LogicalResult
@@ -214,13 +220,19 @@ class CfgIterWhileConv : public mlir::OpRewritePattern<fir::IterWhileOp> {
 public:
   using OpRewritePattern::OpRewritePattern;
 
-  CfgIterWhileConv(mlir::MLIRContext *ctx, bool forceLoopToExecuteOnce)
-      : mlir::OpRewritePattern<fir::IterWhileOp>(ctx) {}
+  CfgIterWhileConv(mlir::MLIRContext *ctx, bool forceLoopToExecuteOnce,
+                   bool setNSW)
+      : mlir::OpRewritePattern<fir::IterWhileOp>(ctx), setNSW(setNSW) {}
 
   mlir::LogicalResult
   matchAndRewrite(fir::IterWhileOp whileOp,
                   mlir::PatternRewriter &rewriter) const override {
     auto loc = whileOp.getLoc();
+    mlir::arith::IntegerOverflowFlags flags{};
+    if (setNSW)
+      flags = bitEnumSet(flags, mlir::arith::IntegerOverflowFlags::nsw);
+    auto iofAttr = mlir::arith::IntegerOverflowFlagsAttr::get(
+        rewriter.getContext(), flags);
 
     // Start by splitting the block containing the 'fir.do_loop' into two parts.
     // The part before will get the init code, the part after will be the end
@@ -248,7 +260,8 @@ class CfgIterWhileConv : public mlir::OpRewritePattern<fir::IterWhileOp> {
     auto *terminator = lastBodyBlock->getTerminator();
     rewriter.setInsertionPointToEnd(lastBodyBlock);
     auto step = whileOp.getStep();
-    mlir::Value stepped = rewriter.create<mlir::arith::AddIOp>(loc, iv, step);
+    mlir::Value stepped =
+        rewriter.create<mlir::arith::AddIOp>(loc, iv, step, iofAttr);
     assert(stepped && "must be a Value");
 
     llvm::SmallVector<mlir::Value> loopCarried;
@@ -305,6 +318,9 @@ class CfgIterWhileConv : public mlir::OpRewritePattern<fir::IterWhileOp> {
     rewriter.replaceOp(whileOp, args);
     return success();
   }
+
+private:
+  bool setNSW;
 };
 
 /// Convert FIR structured control flow ops to CFG ops.
@@ -312,10 +328,13 @@ class CfgConversion : public fir::impl::CFGConversionBase<CfgConversion> {
 public:
   using CFGConversionBase<CfgConversion>::CFGConversionBase;
 
+  CfgConversion(bool setNSW) { this->setNSW = setNSW; }
+
   void runOnOperation() override {
     auto *context = &this->getContext();
     mlir::RewritePatternSet patterns(context);
-    fir::populateCfgConversionRewrites(patterns, this->forceLoopToExecuteOnce);
+    fir::populateCfgConversionRewrites(patterns, this->forceLoopToExecuteOnce,
+                                       this->setNSW);
     mlir::ConversionTarget target(*context);
     target.addLegalDialect<mlir::affine::AffineDialect,
                            mlir::cf::ControlFlowDialect, FIROpsDialect,
@@ -337,7 +356,12 @@ class CfgConversion : public fir::impl::CFGConversionBase<CfgConversion> {
 
 /// Expose conversion rewriters to other passes
 void fir::populateCfgConversionRewrites(mlir::RewritePatternSet &patterns,
-                                        bool forceLoopToExecuteOnce) {
+                                        bool forceLoopToExecuteOnce,
+                                        bool setNSW) {
   patterns.insert<CfgLoopConv, CfgIfConv, CfgIterWhileConv>(
-      patterns.getContext(), forceLoopToExecuteOnce);
+      patterns.getContext(), forceLoopToExecuteOnce, setNSW);
+}
+
+std::unique_ptr<mlir::Pass> fir::createCFGConversionPassWithNSW() {
+  return std::make_unique<CfgConversion>(true);
 }
diff --git a/flang/test/Driver/frontend-forwarding.f90 b/flang/test/Driver/frontend-forwarding.f90
index eac9773ce25c7..35adb47b56861 100644
--- a/flang/test/Driver/frontend-forwarding.f90
+++ b/flang/test/Driver/frontend-forwarding.f90
@@ -19,6 +19,7 @@
 ! RUN:     -fversion-loops-for-stride \
 ! RUN:     -flang-experimental-hlfir \
 ! RUN:     -flang-deprecated-no-hlfir \
+! RUN:     -flang-experimental-integer-overflow \
 ! RUN:     -fno-ppc-native-vector-element-order \
 ! RUN:     -fppc-native-vector-element-order \
 ! RUN:     -mllvm -print-before-all \
@@ -50,6 +51,7 @@
 ! CHECK: "-fversion-loops-for-stride"
 ! CHECK: "-flang-experimental-hlfir"
 ! CHECK: "-flang-deprecated-no-hlfir"
+! CHECK: "-flang-experimental-integer-overflow"
 ! CHECK: "-fno-ppc-native-vector-element-order"
 ! CHECK: "-fppc-native-vector-element-order"
 ! CHECK: "-Rpass"
diff --git a/flang/test/Fir/loop01.fir b/flang/test/Fir/loop01.fir
index 72ca1c3989e45..c1cbb522c378c 100644
--- a/flang/test/Fir/loop01.fir
+++ b/flang/test/Fir/loop01.fir
@@ -1,4 +1,5 @@
 // RUN: fir-opt --split-input-file --cfg-conversion %s | FileCheck %s
+// RUN: fir-opt --split-input-file --cfg-conversion="set-nsw=true" %s | FileCheck %s --check-prefix=NSW
 
 func.func @x(%lb : index, %ub : index, %step : index, %b : i1, %addr : !fir.ref<index>) {
   fir.do_loop %iv = %lb to %ub step %step unordered {
@@ -43,6 +44,34 @@ func.func private @f2() -> i1
 // CHECK:     }
 // CHECK:     func private @f2() -> i1
 
+// NSW:     func @x(%[[VAL_0:.*]]: index, %[[VAL_1:.*]]: index, %[[VAL_2:.*]]: index, %[[VAL_3:.*]]: i1, %[[VAL_4:.*]]: !fir.ref<index>) {
+// NSW:       %[[VAL_5:.*]] = arith.subi %[[VAL_1]], %[[VAL_0]] : index
+// NSW:       %[[VAL_6:.*]] = arith.addi %[[VAL_5]], %[[VAL_2]] : index
+// NSW:       %[[VAL_7:.*]] = arith.divsi %[[VAL_6]], %[[VAL_2]] : index
+// NSW:       br ^bb1(%[[VAL_0]], %[[VAL_7]] : index, index)
+// NSW:     ^bb1(%[[VAL_8:.*]]: index, %[[VAL_9:.*]]: index):
+// NSW:       %[[VAL_10:.*]] = arith.constant 0 : index
+// NSW:       %[[VAL_11:.*]] = arith.cmpi sgt, %[[VAL_9]], %[[VAL_10]] : index
+// NSW:       cond_br %[[VAL_11]], ^bb2, ^bb6
+// NSW:     ^bb2:
+// NSW:       cond_br %[[VAL_3]], ^bb3, ^bb4
+// NSW:     ^bb3:
+// NSW:       fir.store %[[VAL_8]] to %[[VAL_4]] : !fir.ref<index>
+// NSW:       br ^bb5
+// NSW:     ^bb4:
+// NSW:       %[[VAL_12:.*]] = arith.constant 0 : index
+// NSW:       fir.store %[[VAL_12]] to %[[VAL_4]] : !fir.ref<index>
+// NSW:       br ^bb5
+// NSW:     ^bb5:
+// NSW:       %[[VAL_13:.*]] = arith.addi %[[VAL_8]], %[[VAL_2]] overflow<nsw> : index
+// NSW:       %[[VAL_14:.*]] = arith.constant 1 : index
+// NSW:       %[[VAL_15:.*]] = arith.subi %[[VAL_9]], %[[VAL_14]] : index
+// NSW:       br ^bb1(%[[VAL_13]], %[[VAL_15]] : index, index)
+// NSW:     ^bb6:
+// NSW:       return
+// NSW:     }
+// NSW:     func private @f2() -> i1
+
 // -----
 
 func.func @x2(%lo : index, %up : index, %ok : i1) {
@@ -79,6 +108,29 @@ func.func private @f3(i16)
 // CHECK:...
[truncated]

@yus3710-fj yus3710-fj force-pushed the feature/nsw_for_DOvar_inc branch from 26a5d63 to f51cfbe Compare May 9, 2024 11:56
@yus3710-fj
Copy link
Contributor Author

yus3710-fj commented May 10, 2024

Windows build failed with the following message.

fatal error C1060: compiler is out of heap space

I'll try re-running CI with an empty commit because it might have happend by accident.

Copy link
Contributor

@vzakhari vzakhari left a comment

Choose a reason for hiding this comment

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

It looks good to me except maybe for the naming of the lowering option. Thank you for doing this!

@@ -34,5 +34,9 @@ ENUM_LOWERINGOPT(NoPPCNativeVecElemOrder, unsigned, 1, 0)
/// On by default.
ENUM_LOWERINGOPT(Underscoring, unsigned, 1, 1)

/// If true, add nsw flags to arithmetic operations for integer.
/// Off by default.
ENUM_LOWERINGOPT(NoSignedWrap, unsigned, 1, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

NoSignedWrap sounds too generic for an option that only forces setting NSW for the loop variables' increments. Please consider renaming or at least make the scope of NSW assignment clear in the comment above.

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 the review!
I changed the name of the lowering option NoSignedWrap to NSWOnLoopVarInc.

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.

Looks great to me. Thanks for all of your work on this!

Copy link
Contributor

@jeanPerier jeanPerier 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!

This patch adds nsw flag to the increment of do-variables when
a new option is enabled.
NOTE 11.10 in the Fortran 2018 standard says they never overflow.

See also the discussion in 74709 and
discourse post.

empty commit to re-run CI

modify the name of the lowering option and its comment
@yus3710-fj yus3710-fj force-pushed the feature/nsw_for_DOvar_inc branch from 1d64007 to 262824a Compare May 15, 2024 11:59
@yus3710-fj yus3710-fj merged commit 526553b into llvm:main May 16, 2024
3 of 4 checks passed
@yus3710-fj yus3710-fj deleted the feature/nsw_for_DOvar_inc branch May 16, 2024 04:17
yus3710-fj added a commit that referenced this pull request Oct 25, 2024
…o -fno-wrapv (#110063)

nsw is now added to do-variable increment when -fno-wrapv is enabled as
GFortran seems to do.
That means the option introduced by #91579 isn't necessary any more.

Note that the feature of -flang-experimental-integer-overflow is enabled
by default.
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
…o -fno-wrapv (llvm#110063)

nsw is now added to do-variable increment when -fno-wrapv is enabled as
GFortran seems to do.
That means the option introduced by llvm#91579 isn't necessary any more.

Note that the feature of -flang-experimental-integer-overflow is enabled
by default.
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.

5 participants