[flang] add nsw to operations in subscripts #110060
Merged
+206
−10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Yusuke MINATO (yus3710-fj) ChangesThis patch adds nsw to operations when lowering subscripts. See also the discussion in the following discourse post. Patch is 38.63 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/110060.diff 16 Files Affected:
diff --git a/flang/docs/Extensions.md b/flang/docs/Extensions.md
index ed1ef49f8b77a5..3ffd2949e45bf4 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 7594a57a262914..1b24af5f97d1ee 100644
--- a/flang/include/flang/Lower/LoweringOptions.def
+++ b/flang/include/flang/Lower/LoweringOptions.def
@@ -34,8 +34,13 @@ 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.
+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 180e2c8ab33ea2..09f7b892f1ecbe 100644
--- a/flang/include/flang/Optimizer/Builder/FIRBuilder.h
+++ b/flang/include/flang/Optimizer/Builder/FIRBuilder.h
@@ -85,13 +85,16 @@ class FirOpBuilder : public mlir::OpBuilder, public mlir::OpBuilder::Listener {
// 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 @@ class FirOpBuilder : public mlir::OpBuilder, public mlir::OpBuilder::Listener {
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 @@ class FirOpBuilder : public mlir::OpBuilder, public mlir::OpBuilder::Listener {
/// 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 017bfd049d3dc5..1ca265fe7f9870 100644
--- a/flang/lib/Lower/ConvertCall.cpp
+++ b/flang/lib/Lower/ConvertCall.cpp
@@ -2574,9 +2574,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<mlir::Value> isPresent;
if (argLowering) {
fir::ArgLoweringRule argRules =
diff --git a/flang/lib/Lower/ConvertExpr.cpp b/flang/lib/Lower/ConvertExpr.cpp
index 62a7615e1af13c..dd8c43588b7018 100644
--- a/flang/lib/Lower/ConvertExpr.cpp
+++ b/flang/lib/Lower/ConvertExpr.cpp
@@ -1065,7 +1065,15 @@ class ScalarExprLowering {
mlir::Value lhs = fir::getBase(left);
mlir::Value rhs = fir::getBase(right);
assert(lhs.getType() == rhs.getType() && "types must be the same");
- return builder.create<OpTy>(getLoc(), lhs, rhs);
+ if constexpr (std::is_same_v<OpTy, mlir::arith::AddIOp> ||
+ std::is_same_v<OpTy, mlir::arith::SubIOp> ||
+ std::is_same_v<OpTy, mlir::arith::MulIOp>) {
+ auto iofAttr = mlir::arith::IntegerOverflowFlagsAttr::get(
+ builder.getContext(), builder.getIntegerOverflowFlags());
+ return builder.create<OpTy>(getLoc(), lhs, rhs, iofAttr);
+ } else {
+ return builder.create<OpTy>(getLoc(), lhs, rhs);
+ }
}
template <typename OpTy, typename A>
@@ -1397,7 +1405,15 @@ class ScalarExprLowering {
fir::emitFatalError(getLoc(), "subscript triple notation is not scalar");
}
ExtValue genSubscript(const Fortran::evaluate::Subscript &subs) {
- return genval(subs);
+ mlir::arith::IntegerOverflowFlags iofBackup{};
+ if (!converter.getLoweringOptions().getIntegerWrapAround()) {
+ iofBackup = builder.getIntegerOverflowFlags();
+ builder.setIntegerOverflowFlags(mlir::arith::IntegerOverflowFlags::nsw);
+ }
+ auto val = genval(subs);
+ if (!converter.getLoweringOptions().getIntegerWrapAround())
+ builder.setIntegerOverflowFlags(iofBackup);
+ return val;
}
ExtValue gen(const Fortran::evaluate::DataRef &dref) {
diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp
index 1933f38f735b57..5db3b08afb7490 100644
--- a/flang/lib/Lower/ConvertExprToHLFIR.cpp
+++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp
@@ -968,8 +968,17 @@ struct BinaryOp {};
fir::FirOpBuilder &builder, \
const Op &, hlfir::Entity lhs, \
hlfir::Entity rhs) { \
- return hlfir::EntityWithAttributes{ \
- builder.create<GenBinFirOp>(loc, lhs, rhs)}; \
+ if constexpr (std::is_same_v<GenBinFirOp, mlir::arith::AddIOp> || \
+ std::is_same_v<GenBinFirOp, mlir::arith::SubIOp> || \
+ std::is_same_v<GenBinFirOp, mlir::arith::MulIOp>) { \
+ auto iofAttr = mlir::arith::IntegerOverflowFlagsAttr::get( \
+ builder.getContext(), builder.getIntegerOverflowFlags()); \
+ return hlfir::EntityWithAttributes{ \
+ builder.create<GenBinFirOp>(loc, lhs, rhs, iofAttr)}; \
+ } else { \
+ return hlfir::EntityWithAttributes{ \
+ builder.create<GenBinFirOp>(loc, lhs, rhs)}; \
+ } \
} \
};
@@ -1584,9 +1593,12 @@ class HlfirBuilder {
auto rightVal = hlfir::loadTrivialScalar(l, b, rightElement);
return binaryOp.gen(l, b, op.derived(), leftVal, rightVal);
};
+ auto iofBackup = builder.getIntegerOverflowFlags();
+ 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<hlfir::DestroyOp>(loc, elemental); });
@@ -1899,10 +1911,17 @@ class HlfirBuilder {
template <typename T>
hlfir::Entity
HlfirDesignatorBuilder::genSubscript(const Fortran::evaluate::Expr<T> &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/test/Lower/HLFIR/binary-ops.f90 b/flang/test/Lower/HLFIR/binary-ops.f90
index 912cea0f5e0e66..20300cfce4aa5c 100644
--- a/flang/test/Lower/HLFIR/binary-ops.f90
+++ b/flang/test/Lower/HLFIR/binary-ops.f90
@@ -1,5 +1,6 @@
! Test lowering of binary intrinsic operations to HLFIR
! RUN: bbc -emit-hlfir -o - %s 2>&1 | FileCheck %s
+! RUN: bbc -emit-hlfir -o - -fwrapv %s 2>&1 | FileCheck %s --check-prefix=NO-NSW
subroutine int_add(x, y, z)
integer :: x, y, z
@@ -209,6 +210,32 @@ subroutine extremum(c, n, l)
! CHECK: %[[VAL_13:.*]] = arith.cmpi sgt, %[[VAL_11]], %[[VAL_12]] : i64
! CHECK: arith.select %[[VAL_13]], %[[VAL_11]], %[[VAL_12]] : i64
+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:.*]]:2 = hlfir.declare %{{.*}}i"} : (!fir.ref<i32>, !fir.dscope) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK: %[[VAL_5:.*]]:2 = hlfir.declare %{{.*}}j"} : (!fir.ref<i32>, !fir.dscope) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK: %[[VAL_6:.*]]:2 = hlfir.declare %{{.*}}k"} : (!fir.ref<i32>, !fir.dscope) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK: %[[VAL_7:.*]] = fir.load %[[VAL_4]]#0 : !fir.ref<i32>
+! CHECK: %[[VAL_8:.*]] = arith.addi %[[VAL_7]], %c1{{[^ ]*}} overflow<nsw> : i32
+! CHECK: %[[VAL_9:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<i32>
+! CHECK: %[[VAL_10:.*]] = arith.subi %[[VAL_9]], %c2{{[^ ]*}} overflow<nsw> : i32
+! CHECK: %[[VAL_11:.*]] = fir.load %[[VAL_6]]#0 : !fir.ref<i32>
+! CHECK: %[[VAL_12:.*]] = arith.muli %c3{{[^ ]*}}, %[[VAL_11]] overflow<nsw> : i32
+
+! NO-NSW-LABEL: func.func @_QPsubscript(
+! NO-NSW: %[[VAL_4:.*]]:2 = hlfir.declare %{{.*}}i"} : (!fir.ref<i32>, !fir.dscope) -> (!fir.ref<i32>, !fir.ref<i32>)
+! NO-NSW: %[[VAL_5:.*]]:2 = hlfir.declare %{{.*}}j"} : (!fir.ref<i32>, !fir.dscope) -> (!fir.ref<i32>, !fir.ref<i32>)
+! NO-NSW: %[[VAL_6:.*]]:2 = hlfir.declare %{{.*}}k"} : (!fir.ref<i32>, !fir.dscope) -> (!fir.ref<i32>, !fir.ref<i32>)
+! NO-NSW: %[[VAL_7:.*]] = fir.load %[[VAL_4]]#0 : !fir.ref<i32>
+! NO-NSW: %[[VAL_8:.*]] = arith.addi %[[VAL_7]], %c1{{[^ ]*}} : i32
+! NO-NSW: %[[VAL_9:.*]] = fir.load %[[VAL_5]]#0 : !fir.ref<i32>
+! NO-NSW: %[[VAL_10:.*]] = arith.subi %[[VAL_9]], %c2{{[^ ]*}} : i32
+! NO-NSW: %[[VAL_11:.*]] = fir.load %[[VAL_6]]#0 : !fir.ref<i32>
+! NO-NSW: %[[VAL_12:.*]] = arith.muli %c3{{[^ ]*}}, %[[VAL_11]] : i32
+
subroutine cmp_int(l, x, y)
logical :: l
integer :: x, y
diff --git a/flang/test/Lower/HLFIR/vector-subscript-as-value.f90 b/flang/test/Lower/HLFIR/vector-subscript-as-value.f90
index 7161ee088b57a4..850ace1a8d9dec 100644
--- a/flang/test/Lower/HLFIR/vector-subscript-as-value.f90
+++ b/flang/test/Lower/HLFIR/vector-subscript-as-value.f90
@@ -1,6 +1,7 @@
! Test lowering of vector subscript designators outside of the
! assignment left-and side and input IO context.
! RUN: bbc -emit-hlfir -o - -I nw %s 2>&1 | FileCheck %s
+! RUN: bbc -emit-hlfir -o - -I nw -fwrapv %s 2>&1 | FileCheck %s
subroutine foo(x, y)
integer :: x(100)
@@ -150,6 +151,45 @@ subroutine foo4(at1, vector, i, j, k, l, step)
! CHECK: hlfir.yield_element %[[VAL_50]] : f32
! CHECK: }
+subroutine foo5(x, y, z)
+ integer :: x(100)
+ integer(8) :: y(20), z(20)
+ call bar(x(y+z))
+end subroutine
+! CHECK-LABEL: func.func @_QPfoo5(
+! CHECK: %[[VAL_3:.*]] = arith.constant 100 : index
+! CHECK: %[[VAL_4:.*]] = fir.shape %[[VAL_3]] : (index) -> !fir.shape<1>
+! CHECK: %[[VAL_5:.*]]:2 = hlfir.declare %[[VAL_0:[a-z0-9]*]](%[[VAL_4]]) {{.*}}Ex
+! CHECK: %[[VAL_6:.*]] = arith.constant 20 : index
+! CHECK: %[[VAL_7:.*]] = fir.shape %[[VAL_6]] : (index) -> !fir.shape<1>
+! CHECK: %[[VAL_8:.*]]:2 = hlfir.declare %[[VAL_1:[a-z0-9]*]](%[[VAL_7]]) {{.*}}Ey
+! CHECK: %[[VAL_9:.*]] = arith.constant 20 : index
+! CHECK: %[[VAL_10:.*]] = fir.shape %[[VAL_9]] : (index) -> !fir.shape<1>
+! CHECK: %[[VAL_11:.*]]:2 = hlfir.declare %[[VAL_2:[a-z0-9]*]](%[[VAL_10]]) {{.*}}Ez
+! CHECK: %[[VAL_12:.*]] = hlfir.elemental %[[VAL_7]] unordered : (!fir.shape<1>) -> !hlfir.expr<20xi64> {
+! CHECK: ^bb0(%[[VAL_13:.*]]: index):
+! CHECK: %[[VAL_14:.*]] = hlfir.designate %[[VAL_8]]#0 (%[[VAL_13]]) : (!fir.ref<!fir.array<20xi64>>, index) -> !fir.ref<i64>
+! CHECK: %[[VAL_15:.*]] = hlfir.designate %[[VAL_11]]#0 (%[[VAL_13]]) : (!fir.ref<!fir.array<20xi64>>, index) -> !fir.ref<i64>
+! CHECK: %[[VAL_16:.*]] = fir.load %[[VAL_14]] : !fir.ref<i64>
+! CHECK: %[[VAL_17:.*]] = fir.load %[[VAL_15]] : !fir.ref<i64>
+! CHECK: %[[VAL_18:.*]] = arith.addi %[[VAL_16]], %[[VAL_17]] : i64
+! CHECK: hlfir.yield_element %[[VAL_18]] : i64
+! CHECK: }
+! CHECK: %[[VAL_19:.*]] = arith.constant 20 : index
+! CHECK: %[[VAL_20:.*]] = fir.shape %[[VAL_19]] : (index) -> !fir.shape<1>
+! CHECK: %[[VAL_21:.*]] = hlfir.elemental %[[VAL_20]] unordered : (!fir.shape<1>) -> !hlfir.expr<20xi32> {
+! CHECK: ^bb0(%[[VAL_22:.*]]: index):
+! CHECK: %[[VAL_23:.*]] = hlfir.apply %[[VAL_12]], %[[VAL_22]] : (!hlfir.expr<20xi64>, index) -> i64
+! CHECK: %[[VAL_24:.*]] = hlfir.designate %[[VAL_5]]#0 (%[[VAL_23]]) : (!fir.ref<!fir.array<100xi32>>, i64) -> !fir.ref<i32>
+! CHECK: %[[VAL_25:.*]] = fir.load %[[VAL_24]] : !fir.ref<i32>
+! CHECK: hlfir.yield_element %[[VAL_25]] : i32
+! CHECK: }
+! CHECK: %[[VAL_26:.*]]:3 = hlfir.associate %[[VAL_21]](%[[VAL_20]]) {adapt.valuebyref} : (!hlfir.expr<20xi32>, !fir.shape<1>) -> (!fir.ref<!fir.array<20xi32>>, !fir.ref<!fir.array<20xi32>>, i1)
+! CHECK: fir.call @_QPbar(%[[VAL_26]]#1) fastmath<contract> : (!fir.ref<!fir.array<20xi32>>) -> ()
+! CHECK: hlfir.end_associate %[[VAL_26]]#1, %[[VAL_26]]#2 : !fir.ref<!fir.array<20xi32>>, i1
+! CHECK: hlfir.destroy %[[VAL_21]] : !hlfir.expr<20xi32>
+! CHECK: hlfir.destroy %[[VAL_12]] : !hlfir.expr<20xi64>
+
subroutine substring(c, vector, i, j)
character(*) :: c(:)
integer(8) :: vector(:), step, i, j
diff --git a/flang/test/Lower/HLFIR/vector-subscript-lhs.f90 b/flang/test/Lower/HLFIR/vector-subscript-lhs.f90
index 74236b39ebf4f6..1885e7c2fecb0b 100644
--- a/flang/test/Lower/HLFIR/vector-subscript-lhs.f90
+++ b/flang/test/Lower/HLFIR/vector-subscript-lhs.f90
@@ -1,6 +1,7 @@
! Test lowering of vector subscripted designators in assignment
! left-hand sides.
! RUN: bbc -emit-hlfir -o - -I nw %s 2>&1 | FileCheck %s
+! RUN: bbc -emit-hlfir -o - -I nw -fwrapv %s 2>&1 | FileCheck %s
subroutine test_simple(x, vector)
integer(8) :: vector(10)
@@ -97,6 +98,42 @@ subroutine test_nested_vectors(x, vector1, vector2, vector3)
! CHECK: }
! CHECK: }
+subroutine test_added_vectors(x, vector1, vector2)
+ integer(8) :: vector1(10), vector2(10)
+ real :: x(:)
+ x(vector1+vector2) = 42.
+end subroutine
+! CHECK-LABEL: func.func @_QPtest_added_vectors(
+! CHECK: %[[VAL_2:.*]] = arith.constant 10 : index
+! CHECK: %[[VAL_3:.*]] = fir.shape %[[VAL_2]] : (index) -> !fir.shape<1>
+! CHECK: %[[VAL_4:.*]]:2 = hlfir.declare {{.*}}Evector1
+! CHECK: %[[VAL_5:.*]]:2 = hlfir.declare {{.*}}Evector2
+! CHECK: %[[VAL_6:.*]]:2 = hlfir.declare {{.*}}Ex
+! CHECK: hlfir.region_assign {
+! CHECK: %[[VAL_7:.*]] = arith.constant 4.200000e+01 : f32
+! CHECK: hlfir.yield %[[VAL_7]] : f32
+! CHECK: } to {
+! CHECK: %[[VAL_8:.*]] = hlfir.elemental %[[VAL_3]] unordered : (!fir.shape<1>) -> !hlfir.expr<10xi64> {
+! CHECK: ^bb0(%[[VAL_9:.*]]: index):
+! CHECK: %[[VAL_10:.*]] = hlfir.designate %[[VAL_4]]#0 (%[[VAL_9]]) : (!fir.ref<!fir.array<10xi64>>, index) -> !fir.ref<i64>
+! CHECK: %[[VAL_11:.*]] = hlfir.designate %[[VAL_5]]#0 (%[[VAL_9]]) : (!fir.ref<!fir.array<10xi64>>, index) -> !fir.ref<i64>
+! CHECK: %[[VAL_12:.*]] = fir.load %[[VAL_10]] : !fir.ref<i64>
+! CHECK: %[[VAL_13:.*]] = fir.load %[[VAL_11]] : !fir.ref<i64>
+! CHECK: %[[VAL_14:.*]] = arith.addi %[[VAL_12]], %[[VAL_13]] : i64
+! CHECK: hlfir.yield_element %[[VAL_14]] : i64
+! CHECK: }
+! CHECK: %[[VAL_27:.*]] = arith.constant 10 : index
+! CHECK: %[[VAL_28:.*]] = fir.shape %[[VAL_27]] : (index) -> !fir.shape<1>
+! CHECK: hlfir.elemental_addr %[[VAL_28]] unordered : !fir.shape<1> {
+! CHECK: ^bb0(%[[VAL_29:.*]]: index):
+! CHECK: %[[VAL_31:.*]] = hlfir.apply %[[VAL_8]], %[[VAL_29]] : (!hlfir.expr<10xi64>, index) -> i64
+! CHECK: %[[VAL_32:.*]] = hlfir.designate %[[VAL_6]]#0 (%[[VAL_31]]) : (!fir.box<!fir.array<?xf32>>, i64) -> !fir.ref<f32>
+! CHECK: hlfir.yield %[[VAL_32]] : !fir.ref<f32>
+! CHECK: } cleanup {
+! CHECK: hlfir.destroy %[[VAL_8]] : !hlfir.expr<10xi64>
+! CHECK: }
+! CHECK: }
+
subroutine test_substring(x, vector)
integer(8) :: vector(10), ifoo, ibar
external :: ifoo, ibar
diff --git a/flang/test/Lower/Intrinsics/bge.f90 b/flang/test/Lower/Intrinsics/bge.f90
index 19ddf7d95c3881..ccd5a7152f4fbb 100644
--- a/flang/test/Lower/Intrinsics/bge.f90
+++ b/flang/test/Lower/Intrinsics/bge.f90
@@ -156,3 +156,20 @@ subroutine bge_test11(c)
! CHECK: %[[V:.*]] = fir.convert %[[R]] : (i1) -> !fir.logical<4>
! CHECK: fir.store %[[V]] to %[[C]] : !fir.ref<!fir.logical<4>>
end subroutine bge_test11
+
+! CHECK-LABEL: bge_test12
+! CHECK-SAME: %[[A:.*]]: !fir.ref<i32>{{.*}}, %[[B:.*]]: !fir.ref<i32>{{.*}}, %[[C:.*]]: !fir.ref<!fir.logical<4>>{{.*}}
+subroutine bge_test12(a, b, c)
+ integer :: a, b
+ logical :: c
+ ! CHECK: %[[A_VAL:.*]] = fir.load %[[A]] : !fir.ref<i32>
+ ! CHECK: %[[B_VAL:.*]] = fir.load %[[B]] : !fir.ref<i32>
+ ! CHECK: %[[LHS:.*]] = arith.addi %[[A_VAL]], %[[B_VAL]] : i32
+ ! CHECK: %[[A_VAL2:.*]] = fir.load %[[A]] : !fir.ref<i32>
+ ! CHECK: %[[B_VAL2:.*]] = fir.load %[[B]] : !fir.ref<i32>
+ ! CHECK: %[[RHS:.*]] = arith.subi %[[A_VAL2]], %[[B_VAL2]] : i32
+ c = bge(a+b, a-b)
+ ! CHECK: %[[C_CMP:.*]] = arith.cmpi uge, %[[LHS]], %[[RHS]] : i32
+ ! CHECK: %[[C_VAL:.*]] = fir.convert %[[C_CMP]] : (i1) -> !fir.logical<4>
+ ! CHECK: fir.store %[[C_VAL]] to %[[C]] : !fir.ref<!fir.logical<4>>
+end subroutine bge_test12
diff --git a/flang/test/Lower/Intrinsics/bgt.f90 b/flang/test/Lower/Intrinsics/bgt.f90
index fea8611c17014e..18c2afeae13d8f 100644
--- a/flang/test/Lower/Intrinsics/bgt.f90
+++ b/flang/test/Lower/Intr...
[truncated]
|
jeanPerier
reviewed
Sep 26, 2024
There was a problem hiding this comment.
Thank you very much for taking the time to build this carefully crafted solution.
I am wondering if it would be OK to do like for the fastmath flags inside the builder directly here using the ArithIntegerOverflowFlagsInterface so that this solution would easily usable in other contexts in the future and better enclosed in the builder.
But it would add the flags on more arith ops generated by lowering outside of the contexts of evaluate::AddIOp, SubIOp, and MulIOp lowering. I would tend to say these compiler generated ops in lowering are not expected to overflow, but it is hard to audit, so I understand why you took the current approach which is more conservative. I am mostly writing that here to double check with other reviewers and document the choice for git blamers like me.
flang/test/Lower/array.f90
Outdated
| @@ -1,4 +1,5 @@ | |||
| ! RUN: bbc -hlfir=false -o - %s | FileCheck %s | |||
| ! RUN: bbc -hlfir=false -fwrapv -o - %s | FileCheck %s --check-prefix=NO-NSW | |||
There was a problem hiding this comment.
There is no point adding test an updating the non HLFIR part. It is deprecated/legacy. The only reason it is still there is because many tests needs to be ported to HLFIR to ensure coverage on non HLFIR specific lowering utilities.
There was a problem hiding this comment.
I'd just like to confirm what you mean.
nsw is also added to operations when HLFIR lowering is disabled, and I added some tests for that. Do you mean only the tests are unnecessary, or the implementation for non HLFIR is also unnecessary?
There was a problem hiding this comment.
Both, the none HLFIR lowering implementation is not maintained and will go away as soon as the hlfir=false tests are ported.
There was a problem hiding this comment.
Thank you for your reply. I fixed it.
| integer(8) :: y(20), z(20) | ||
| call bar(x(y+z)) | ||
| end subroutine | ||
| ! CHECK-LABEL: func.func @_QPfoo5( |
There was a problem hiding this comment.
If the goal of the test is to show that overflow is no generated here, I think it is simpler and more expressive to just go for:
! Test that -fwrapv does not add <nsw> flag to arith ops on vector subscripts.
! CHECK-LABEL: func.func @_QPfoo5(
! CHECK-NOT: overflow<nsw>
! CHECK: return
There was a problem hiding this comment.
Thank you for showing me a better way. I'll modify it.
flang/test/Lower/Intrinsics/bge.f90
Outdated
| @@ -156,3 +156,20 @@ subroutine bge_test11(c) | |||
| ! CHECK: %[[V:.*]] = fir.convert %[[R]] : (i1) -> !fir.logical<4> | |||
| ! CHECK: fir.store %[[V]] to %[[C]] : !fir.ref<!fir.logical<4>> | |||
| end subroutine bge_test11 | |||
|
|
|||
| ! CHECK-LABEL: bge_test12 | |||
There was a problem hiding this comment.
Nit: don't you need to add the -fwrap flag to the RUN line for test to be relevant?
Also, to avoid updating the legacy lowering code just for the tests. I would advise adding all the new tests in a new test/Lower/frwap-option.f90 file run with HFLIR.
I think you can use CHECK-NOT here to too make the tests smaller.
There was a problem hiding this comment.
Thank you for your advice. I'll create another test file and add -fwrapv to the RUN command.
tblah
reviewed
Sep 30, 2024
|
Thank you for pointing it out. I fixed it. |
tblah
approved these changes
Oct 1, 2024
This patch adds nsw to operations when lowering subscripts. See also the discussion in the following discourse post. https://discourse.llvm.org/t/rfc-add-nsw-flags-to-arithmetic-integer-operations-using-the-option-fno-wrapv/77584/9
yus3710-fj
force-pushed
the
nsw-on-subscripts
branch
from
2213126
to
1874326
Compare
|
Rebased. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds nsw to operations when lowering subscripts.
See also the discussion in the following discourse post.
https://discourse.llvm.org/t/rfc-add-nsw-flags-to-arithmetic-integer-operations-using-the-option-fno-wrapv/77584/9