use setCommonAttributes rather than gen

llvm · Oct 1, 2024 · 2213126 · 2213126
1 parent a267973
commit 2213126
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 19 deletions.
diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp
@@ -968,17 +968,8 @@ struct BinaryOp {};
                                            fir::FirOpBuilder &builder,         \
                                            const Op &, hlfir::Entity lhs,      \
                                            hlfir::Entity 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)};                       \
-      }                                                                        \
+      return hlfir::EntityWithAttributes{                                      \
+          builder.create<GenBinFirOp>(loc, lhs, rhs)};                         \
     }                                                                          \
   };
 

diff --git 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<mlir::arith::ArithFastMathInterface>(*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<mlir::arith::ArithIntegerOverflowFlagsInterface>(*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/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<fir::UndefOp>(loc, intTy);
+
+  // Test that IntegerOverflowFlags is 'none' by default.
+  mlir::Operation *op1 = builder.create<mlir::arith::AddIOp>(loc, arg, arg);
+  auto op1_iofi =
+      mlir::dyn_cast_or_null<mlir::arith::ArithIntegerOverflowFlagsInterface>(
+          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<mlir::arith::AddIOp>(loc, arg, arg);
+  auto op2_iofi =
+      mlir::dyn_cast_or_null<mlir::arith::ArithIntegerOverflowFlagsInterface>(
+          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<mlir::arith::AddIOp>(loc, arg, arg);
+  auto op3_iofi =
+      mlir::dyn_cast_or_null<mlir::arith::ArithIntegerOverflowFlagsInterface>(
+          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<mlir::arith::AddIOp>(loc, arg, arg);
+  auto op4_iofi =
+      mlir::dyn_cast_or_null<mlir::arith::ArithIntegerOverflowFlagsInterface>(
+          op4);
+  EXPECT_TRUE(op4_iofi);
+  auto op4_ioff = op4_iofi.getOverflowAttr().getValue();
+  EXPECT_EQ(op4_ioff, nsw);
+}