diff --git a/flang/docs/Extensions.md b/flang/docs/Extensions.md index ed1ef49f8b77..3ffd2949e45b 100644 --- a/flang/docs/Extensions.md +++ b/flang/docs/Extensions.md @@ -780,6 +780,12 @@ character j print *, [(j,j=1,10)] ``` +* The Fortran standard doesn't mention integer overflow explicitly. In many cases, + however, integer overflow makes programs non-conforming. + F18 follows other widely-used Fortran compilers. Specifically, f18 assumes + integer overflow never occurs in address calculations and increment of + do-variable unless the option `-fwrapv` is enabled. + ## De Facto Standard Features * `EXTENDS_TYPE_OF()` returns `.TRUE.` if both of its arguments have the diff --git a/flang/include/flang/Lower/LoweringOptions.def b/flang/include/flang/Lower/LoweringOptions.def index 7594a57a2629..d3f17c3f939c 100644 --- a/flang/include/flang/Lower/LoweringOptions.def +++ b/flang/include/flang/Lower/LoweringOptions.def @@ -34,8 +34,14 @@ ENUM_LOWERINGOPT(NoPPCNativeVecElemOrder, unsigned, 1, 0) /// On by default. 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) + /// If true, add nsw flags to loop variable increments. /// Off by default. +/// TODO: integrate this option with the above ENUM_LOWERINGOPT(NSWOnLoopVarInc, unsigned, 1, 0) #undef LOWERINGOPT diff --git a/flang/include/flang/Optimizer/Builder/FIRBuilder.h b/flang/include/flang/Optimizer/Builder/FIRBuilder.h index 180e2c8ab33e..09f7b892f1ec 100644 --- a/flang/include/flang/Optimizer/Builder/FIRBuilder.h +++ b/flang/include/flang/Optimizer/Builder/FIRBuilder.h @@ -85,13 +85,16 @@ public: // The listener self-reference has to be updated in case of copy-construction. FirOpBuilder(const FirOpBuilder &other) : OpBuilder(other), OpBuilder::Listener(), kindMap{other.kindMap}, - fastMathFlags{other.fastMathFlags}, symbolTable{other.symbolTable} { + fastMathFlags{other.fastMathFlags}, + integerOverflowFlags{other.integerOverflowFlags}, + symbolTable{other.symbolTable} { setListener(this); } FirOpBuilder(FirOpBuilder &&other) : OpBuilder(other), OpBuilder::Listener(), kindMap{std::move(other.kindMap)}, fastMathFlags{other.fastMathFlags}, + integerOverflowFlags{other.integerOverflowFlags}, symbolTable{other.symbolTable} { setListener(this); } @@ -521,6 +524,18 @@ public: return fmfString; } + /// Set default IntegerOverflowFlags value for all operations + /// supporting mlir::arith::IntegerOverflowFlagsAttr that will be created + /// by this builder. + void setIntegerOverflowFlags(mlir::arith::IntegerOverflowFlags flags) { + integerOverflowFlags = flags; + } + + /// Get current IntegerOverflowFlags value. + mlir::arith::IntegerOverflowFlags getIntegerOverflowFlags() const { + return integerOverflowFlags; + } + /// Dump the current function. (debug) LLVM_DUMP_METHOD void dumpFunc(); @@ -547,6 +562,10 @@ private: /// mlir::arith::FastMathAttr. mlir::arith::FastMathFlags fastMathFlags{}; + /// IntegerOverflowFlags that need to be set for operations that support + /// mlir::arith::IntegerOverflowFlagsAttr. + mlir::arith::IntegerOverflowFlags integerOverflowFlags{}; + /// fir::GlobalOp and func::FuncOp symbol table to speed-up /// lookups. mlir::SymbolTable *symbolTable = nullptr; diff --git a/flang/lib/Lower/ConvertCall.cpp b/flang/lib/Lower/ConvertCall.cpp index ee5eb225f0d7..59f29c409c79 100644 --- a/flang/lib/Lower/ConvertCall.cpp +++ b/flang/lib/Lower/ConvertCall.cpp @@ -2570,9 +2570,26 @@ genIntrinsicRef(const Fortran::evaluate::SpecificIntrinsic *intrinsic, hlfir::Entity{*var}, /*isPresent=*/std::nullopt}); continue; } + // arguments of bitwise comparison functions may not have nsw flag + // even if -fno-wrapv is enabled + mlir::arith::IntegerOverflowFlags iofBackup{}; + auto isBitwiseComparison = [](const std::string intrinsicName) -> bool { + if (intrinsicName == "bge" || intrinsicName == "bgt" || + intrinsicName == "ble" || intrinsicName == "blt") + return true; + return false; + }; + if (isBitwiseComparison(callContext.getProcedureName())) { + iofBackup = callContext.getBuilder().getIntegerOverflowFlags(); + callContext.getBuilder().setIntegerOverflowFlags( + mlir::arith::IntegerOverflowFlags::none); + } auto loweredActual = Fortran::lower::convertExprToHLFIR( loc, callContext.converter, *expr, callContext.symMap, callContext.stmtCtx); + if (isBitwiseComparison(callContext.getProcedureName())) + callContext.getBuilder().setIntegerOverflowFlags(iofBackup); + std::optional isPresent; if (argLowering) { fir::ArgLoweringRule argRules = diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp index 1933f38f735b..8a1effd75a49 100644 --- a/flang/lib/Lower/ConvertExprToHLFIR.cpp +++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp @@ -1584,9 +1584,14 @@ private: auto rightVal = hlfir::loadTrivialScalar(l, b, rightElement); return binaryOp.gen(l, b, op.derived(), leftVal, rightVal); }; + auto iofBackup = builder.getIntegerOverflowFlags(); + // nsw is never added to operations on vector subscripts + // even if -fno-wrapv is enabled. + builder.setIntegerOverflowFlags(mlir::arith::IntegerOverflowFlags::none); mlir::Value elemental = hlfir::genElementalOp(loc, builder, elementType, shape, typeParams, genKernel, /*isUnordered=*/true); + builder.setIntegerOverflowFlags(iofBackup); fir::FirOpBuilder *bldr = &builder; getStmtCtx().attachCleanup( [=]() { bldr->create(loc, elemental); }); @@ -1899,10 +1904,17 @@ private: template hlfir::Entity HlfirDesignatorBuilder::genSubscript(const Fortran::evaluate::Expr &expr) { + fir::FirOpBuilder &builder = getBuilder(); + mlir::arith::IntegerOverflowFlags iofBackup{}; + if (!getConverter().getLoweringOptions().getIntegerWrapAround()) { + iofBackup = builder.getIntegerOverflowFlags(); + builder.setIntegerOverflowFlags(mlir::arith::IntegerOverflowFlags::nsw); + } auto loweredExpr = HlfirBuilder(getLoc(), getConverter(), getSymMap(), getStmtCtx()) .gen(expr); - fir::FirOpBuilder &builder = getBuilder(); + if (!getConverter().getLoweringOptions().getIntegerWrapAround()) + builder.setIntegerOverflowFlags(iofBackup); // Skip constant conversions that litters designators and makes generated // IR harder to read: directly use index constants for constant subscripts. mlir::Type idxTy = builder.getIndexType(); diff --git a/flang/lib/Optimizer/Builder/FIRBuilder.cpp b/flang/lib/Optimizer/Builder/FIRBuilder.cpp index 539235f01f5f..9c26a3611855 100644 --- a/flang/lib/Optimizer/Builder/FIRBuilder.cpp +++ b/flang/lib/Optimizer/Builder/FIRBuilder.cpp @@ -768,14 +768,23 @@ mlir::Value fir::FirOpBuilder::genAbsentOp(mlir::Location loc, void fir::FirOpBuilder::setCommonAttributes(mlir::Operation *op) const { auto fmi = mlir::dyn_cast(*op); - if (!fmi) - return; - // TODO: use fmi.setFastMathFlagsAttr() after D137114 is merged. - // For now set the attribute by the name. - llvm::StringRef arithFMFAttrName = fmi.getFastMathAttrName(); - if (fastMathFlags != mlir::arith::FastMathFlags::none) - op->setAttr(arithFMFAttrName, mlir::arith::FastMathFlagsAttr::get( - op->getContext(), fastMathFlags)); + if (fmi) { + // TODO: use fmi.setFastMathFlagsAttr() after D137114 is merged. + // For now set the attribute by the name. + llvm::StringRef arithFMFAttrName = fmi.getFastMathAttrName(); + if (fastMathFlags != mlir::arith::FastMathFlags::none) + op->setAttr(arithFMFAttrName, mlir::arith::FastMathFlagsAttr::get( + op->getContext(), fastMathFlags)); + } + auto iofi = + mlir::dyn_cast(*op); + if (iofi) { + llvm::StringRef arithIOFAttrName = iofi.getIntegerOverflowAttrName(); + if (integerOverflowFlags != mlir::arith::IntegerOverflowFlags::none) + op->setAttr(arithIOFAttrName, + mlir::arith::IntegerOverflowFlagsAttr::get( + op->getContext(), integerOverflowFlags)); + } } void fir::FirOpBuilder::setFastMathFlags( diff --git a/flang/test/Lower/nsw.f90 b/flang/test/Lower/nsw.f90 new file mode 100644 index 000000000000..84435b713304 --- /dev/null +++ b/flang/test/Lower/nsw.f90 @@ -0,0 +1,61 @@ +! RUN: bbc -emit-fir %s -o - | FileCheck %s +! RUN: bbc -emit-fir -fwrapv %s -o - | FileCheck %s --check-prefix=NO-NSW + +! NO-NSW-NOT: overflow + +subroutine subscript(a, i, j, k) + integer :: a(:,:,:), i, j, k + a(i+1, j-2, k*3) = 5 +end subroutine +! CHECK-LABEL: func.func @_QPsubscript( +! CHECK: %[[VAL_4:.*]] = arith.constant 3 : i32 +! CHECK: %[[VAL_5:.*]] = arith.constant 2 : i32 +! CHECK: %[[VAL_6:.*]] = arith.constant 1 : i32 +! CHECK: %[[VAL_9:.*]] = fir.declare %{{.*}}a"} : (!fir.box>, !fir.dscope) -> !fir.box> +! CHECK: %[[VAL_10:.*]] = fir.rebox %[[VAL_9]] : (!fir.box>) -> !fir.box> +! CHECK: %[[VAL_11:.*]] = fir.declare %{{.*}}i"} : (!fir.ref, !fir.dscope) -> !fir.ref +! CHECK: %[[VAL_12:.*]] = fir.declare %{{.*}}j"} : (!fir.ref, !fir.dscope) -> !fir.ref +! CHECK: %[[VAL_13:.*]] = fir.declare %{{.*}}k"} : (!fir.ref, !fir.dscope) -> !fir.ref +! CHECK: %[[VAL_14:.*]] = fir.load %[[VAL_11]] : !fir.ref +! CHECK: %[[VAL_15:.*]] = arith.addi %[[VAL_14]], %[[VAL_6]] overflow : i32 +! CHECK: %[[VAL_16:.*]] = fir.convert %[[VAL_15]] : (i32) -> i64 +! CHECK: %[[VAL_17:.*]] = fir.load %[[VAL_12]] : !fir.ref +! CHECK: %[[VAL_18:.*]] = arith.subi %[[VAL_17]], %[[VAL_5]] overflow : i32 +! CHECK: %[[VAL_19:.*]] = fir.convert %[[VAL_18]] : (i32) -> i64 +! CHECK: %[[VAL_20:.*]] = fir.load %[[VAL_13]] : !fir.ref +! CHECK: %[[VAL_21:.*]] = arith.muli %[[VAL_20]], %[[VAL_4]] overflow : i32 +! CHECK: %[[VAL_22:.*]] = fir.convert %[[VAL_21]] : (i32) -> i64 +! CHECK: %[[VAL_23:.*]] = fir.array_coor %[[VAL_10]] %[[VAL_16]], %[[VAL_19]], %[[VAL_22]] : + +! Test that nsw is never added to arith ops +! on vector subscripts. +subroutine vector_subscript_as_value(x, y, z) + integer :: x(100) + integer(8) :: y(20), z(20) + call bar(x(y+z)) +end subroutine +! CHECK-LABEL: func.func @_QPvector_subscript_as_value( +! CHECK-NOT: overflow +! CHECK: return + +subroutine vector_subscript_lhs(x, vector1, vector2) + integer(8) :: vector1(10), vector2(10) + real :: x(:) + x(vector1+vector2) = 42. +end subroutine +! CHECK-LABEL: func.func @_QPvector_subscript_lhs( +! CHECK-NOT: overflow +! CHECK: return + +! Test that nsw is never added to arith ops +! on arguments of bitwise comparison intrinsics. +subroutine bitwise_comparison(a, b) + integer :: a, b + print *, bge(a+b, a-b) + print *, bgt(a+b, a-b) + print *, ble(a+b, a-b) + print *, blt(a+b, a-b) +end subroutine +! CHECK-LABEL: func.func @_QPbitwise_comparison( +! CHECK-NOT: overflow +! CHECK: return diff --git a/flang/tools/bbc/bbc.cpp b/flang/tools/bbc/bbc.cpp index 0a008d577cc2..3a05f5f98448 100644 --- a/flang/tools/bbc/bbc.cpp +++ b/flang/tools/bbc/bbc.cpp @@ -228,6 +228,12 @@ static llvm::cl::opt llvm::cl::desc("Override host target triple"), llvm::cl::init("")); +static llvm::cl::opt integerWrapAround( + "fwrapv", + llvm::cl::desc("Treat signed integer overflow as two's complement"), + llvm::cl::init(false)); + +// TODO: integrate this option with the above static llvm::cl::opt setNSW("integer-overflow", llvm::cl::desc("add nsw flag to internal operations"), @@ -373,6 +379,7 @@ static llvm::LogicalResult convertFortranSourceToMLIR( Fortran::lower::LoweringOptions loweringOptions{}; loweringOptions.setNoPPCNativeVecElemOrder(enableNoPPCNativeVecElemOrder); loweringOptions.setLowerToHighLevelFIR(useHLFIR || emitHLFIR); + loweringOptions.setIntegerWrapAround(integerWrapAround); loweringOptions.setNSWOnLoopVarInc(setNSW); std::vector envDefaults = {}; constexpr const char *tuneCPU = ""; diff --git a/flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp b/flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp index e5e5454ee88a..f63afe413768 100644 --- a/flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp +++ b/flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp @@ -585,3 +585,62 @@ TEST_F(FIRBuilderTest, genArithFastMath) { auto op4_fmf = op4_fmi.getFastMathFlagsAttr().getValue(); EXPECT_EQ(op4_fmf, FMF1); } + +TEST_F(FIRBuilderTest, genArithIntegerOverflow) { + auto builder = getBuilder(); + auto ctx = builder.getContext(); + auto loc = builder.getUnknownLoc(); + + auto intTy = IntegerType::get(ctx, 32); + auto arg = builder.create(loc, intTy); + + // Test that IntegerOverflowFlags is 'none' by default. + mlir::Operation *op1 = builder.create(loc, arg, arg); + auto op1_iofi = + mlir::dyn_cast_or_null( + op1); + EXPECT_TRUE(op1_iofi); + auto op1_ioff = op1_iofi.getOverflowAttr().getValue(); + EXPECT_EQ(op1_ioff, arith::IntegerOverflowFlags::none); + + // Test that the builder is copied properly. + fir::FirOpBuilder builder_copy(builder); + + arith::IntegerOverflowFlags nsw = arith::IntegerOverflowFlags::nsw; + builder.setIntegerOverflowFlags(nsw); + arith::IntegerOverflowFlags nuw = arith::IntegerOverflowFlags::nuw; + builder_copy.setIntegerOverflowFlags(nuw); + + // Modifying IntegerOverflowFlags for the copy must not affect the original + // builder. + mlir::Operation *op2 = builder.create(loc, arg, arg); + auto op2_iofi = + mlir::dyn_cast_or_null( + op2); + EXPECT_TRUE(op2_iofi); + auto op2_ioff = op2_iofi.getOverflowAttr().getValue(); + EXPECT_EQ(op2_ioff, nsw); + + // Modifying IntegerOverflowFlags for the original builder must not affect the + // copy. + mlir::Operation *op3 = + builder_copy.create(loc, arg, arg); + auto op3_iofi = + mlir::dyn_cast_or_null( + op3); + EXPECT_TRUE(op3_iofi); + auto op3_ioff = op3_iofi.getOverflowAttr().getValue(); + EXPECT_EQ(op3_ioff, nuw); + + // Test that the builder copy inherits IntegerOverflowFlags from the original. + fir::FirOpBuilder builder_copy2(builder); + + mlir::Operation *op4 = + builder_copy2.create(loc, arg, arg); + auto op4_iofi = + mlir::dyn_cast_or_null( + op4); + EXPECT_TRUE(op4_iofi); + auto op4_ioff = op4_iofi.getOverflowAttr().getValue(); + EXPECT_EQ(op4_ioff, nsw); +}