[flang] Remove non-constant folding for multiply

[shraiysh]

Summary

This folding is already done by the canonicalizer in mlir and there are
no dedicated tests for non-constant folding in flang. This has also not
been implemented for other binary operations like add, subtract and
divide - all of which are handled by the canonicalizer. Hence, removing
this change.

This does not affect constant folding and the semantic checks associated
with that.

[klausler]

Why not add other missing simple rewrites rather than remove this one?

[shraiysh]

Why not add other missing simple rewrites rather than remove this one?

This already being done by the fir-opt canonicalization. It would be code duplication (functionality wise). That's the main reason to remove this. The canonicalization in arith dialect handles many more cases than we already have. Unlike constant folding, this is not required by any of the semantic checks and so, pushing it later in the pipeline and relying on MLIR doesn't hurt.

A minor reason is that there is no proper way to test this for general usage. As we discussed on slack, there is a way to test this only in modules only when it is used in specification based initialization, but there is no general test for this and its hard to figure out how and where to write such a test.

[klausler]

In D126645#3545990, @shraiysh wrote:

Why not add other missing simple rewrites rather than remove this one?

This already being done by the fir-opt canonicalization. It would be code duplication (functionality wise). That's the main reason to remove this. The canonicalization in arith dialect handles many more cases than we already have. Unlike constant folding, this is not required by any of the semantic checks and so, pushing it later in the pipeline and relying on MLIR doesn't hurt.

A minor reason is that there is no proper way to test this for general usage. As we discussed on slack, there is a way to test this only in modules only when it is used in specification based initialization, but there is no general test for this and its hard to figure out how and where to write such a test.

There's some use cases for rewriting 0 * ... as 0 to avoid spurious diagnostics from semantics in the ... part; unlike C/C++, a Fortran compiler is allowed this rewrite even when there's function calls with potential side effects in the ... part. Also MERGE(t,f,x) when x is known.

[shraiysh]

In D126645#3546069, @klausler wrote:

There's some use cases for rewriting 0 * ... as 0 to avoid spurious diagnostics from semantics in the ... part; unlike C/C++, a Fortran compiler is allowed this rewrite even when there's function calls with potential side effects in the ... part. Also MERGE(t,f,x) when x is known.

In that case, this should be done only for multiplication with zero (*0) in flang, right? Is it okay if we rely on mlir for the remaining simplifications, like +0, -0, /1, *1, etc.?

[klausler]

In D126645#3546860, @shraiysh wrote:

In D126645#3546069, @klausler wrote:

There's some use cases for rewriting 0 * ... as 0 to avoid spurious diagnostics from semantics in the ... part; unlike C/C++, a Fortran compiler is allowed this rewrite even when there's function calls with potential side effects in the ... part. Also MERGE(t,f,x) when x is known.

In that case, this should be done only for multiplication with zero (*0) in flang, right? Is it okay if we rely on mlir for the remaining simplifications, like +0, -0, /1, *1, etc.?

I suppose so, but I've found it to be a good idea to simplify and optimize programs as early as possible, in general. Do what you like, it's your tree.

[shraiysh]

Abandoning this.

flang/lib/Evaluate/fold-implementation.h

if (auto folded{OperandsAreConstants(x)}) {
     } else {
       auto product{folded->first.Multiply(folded->second, context.rounding())};
       RealFlagWarnings(context, product.flags, "multiplication");
       if (context.flushSubnormalsToZero()) {
         product.value = product.value.FlushSubnormalToZero();
       }
       return Expr<T>{Constant<T>{product.value}};
     }
-  } else if constexpr (T::category == TypeCategory::Integer) {
-    if (auto c{GetScalarConstantValue<T>(x.right())}) {
-      x.right() = std::move(x.left());
-      x.left() = Expr<T>{std::move(*c)};
-    }
-    if (auto c{GetScalarConstantValue<T>(x.left())}) {
-      if (c->IsZero()) {
-        return std::move(x.left());
-      } else if (c->CompareSigned(Scalar<T>{1}) == Ordering::Equal) {
-        return std::move(x.right());
-      } else if (c->CompareSigned(Scalar<T>{-1}) == Ordering::Equal) {
-        return Expr<T>{Negate<T>{std::move(x.right())}};
-      }
-    }
   }
   return Expr<T>{std::move(x)};
 }
 
 template <typename T>
 Expr<T> FoldOperation(FoldingContext &context, Divide<T> &&x) {
   if (auto array{ApplyElementwise(context, x)}) {
     return *array;

flang/test/Evaluate/rewrite01.f90

subroutine ubound_test(x, n, m)
   !CHECK: PRINT *, ubound(y)
   print *, ubound(y)
   !CHECK: PRINT *, ubound(y,1_4)
   print *, ubound(y, 1)
 end subroutine
 
 subroutine size_test(x, n, m)
   integer :: x(n, m)
-  !CHECK: PRINT *, int(size(x,dim=1,kind=8)*size(x,dim=2,kind=8),kind=4)
+  !CHECK: PRINT *, int(1_8*size(x,dim=1,kind=8)*size(x,dim=2,kind=8),kind=4)
   print *, size(x)
   !CHECK: PRINT *, size(returns_array(n,m))
   print *, size(returns_array(n, m))
   !CHECK: PRINT *, size(returns_array(n,m),dim=1_4)
   print *, size(returns_array(n, m), dim=1)
   !CHECK: PRINT *, size(returns_array_2(m))
   print *, size(returns_array_2(m))
   !CHECK: PRINT *, 42_8

flang/test/Lower/OpenMP/omp-wsloop-chunks.f90

do i=1, 9
   print*, i*2
 
 ! CHECK:         %[[VAL_14:.*]] = arith.constant 1 : i32
 ! CHECK:         %[[VAL_15:.*]] = arith.constant 9 : i32
 ! CHECK:         %[[VAL_16:.*]] = arith.constant 1 : i32
 ! CHECK:         %[[VAL_17:.*]] = arith.constant 4 : i32
 ! CHECK:         omp.wsloop   schedule(static = %[[VAL_17]] : i32) nowait for  (%[[VAL_18:.*]]) : i32 = (%[[VAL_14]]) to (%[[VAL_15]]) inclusive step (%[[VAL_16]]) {
 ! CHECK:           %[[VAL_24:.*]] = arith.constant 2 : i32
-! CHECK:           %[[VAL_25:.*]] = arith.muli %[[VAL_24]], %[[VAL_18]] : i32
+! CHECK:           %[[VAL_25:.*]] = arith.muli %[[VAL_18]], %[[VAL_24]] : i32
 ! CHECK:           {{.*}} = fir.call @_FortranAioOutputInteger32({{.*}}, %[[VAL_25]]) : (!fir.ref<i8>, i32) -> i1
 ! CHECK:           omp.yield
 ! CHECK:         }
   
 end do
 !$OMP END DO NOWAIT
 chunk = 6
 !$OMP DO SCHEDULE(static, chunk)
 
 do i=1, 9
    print*, i*3
 end do
 !$OMP END DO NOWAIT
 ! CHECK:         %[[VAL_28:.*]] = arith.constant 6 : i32
 ! CHECK:         fir.store %[[VAL_28]] to %[[VAL_0]] : !fir.ref<i32>
 ! CHECK:         %[[VAL_29:.*]] = arith.constant 1 : i32
 ! CHECK:         %[[VAL_30:.*]] = arith.constant 9 : i32
 ! CHECK:         %[[VAL_31:.*]] = arith.constant 1 : i32
 ! CHECK:         %[[VAL_32:.*]] = fir.load %[[VAL_0]] : !fir.ref<i32>
 ! CHECK:         omp.wsloop   schedule(static = %[[VAL_32]] : i32) nowait for  (%[[VAL_33:.*]]) : i32 = (%[[VAL_29]]) to (%[[VAL_30]]) inclusive step (%[[VAL_31]]) {
 ! CHECK:           %[[VAL_39:.*]] = arith.constant 3 : i32
-! CHECK:           %[[VAL_40:.*]] = arith.muli %[[VAL_39]], %[[VAL_33]] : i32
+! CHECK:           %[[VAL_40:.*]] = arith.muli %[[VAL_33]], %[[VAL_39]] : i32
 ! CHECK:           {{.*}} = fir.call @_FortranAioOutputInteger32({{.*}}, %[[VAL_40]]) : (!fir.ref<i8>, i32) -> i1
 ! CHECK:           omp.yield
 ! CHECK:         }
 ! CHECK:         return
 ! CHECK:       }
 
 end

flang/test/Lower/variable-inquiries.f90

subroutine issue844()
   ! Verify that evaluate::DescriptorInquiry are made using the symbol mapped
   ! in lowering (the use associated one, and not directly the ultimate
   ! symbol).
 
   ! CHECK: %[[a:.*]] = fir.address_of(@_QMinquiredEa) : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>
   ! CHECK: %[[box_load:.*]] = fir.load %[[a]] : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>
   ! CHECK: %[[dim:.*]]:3 = fir.box_dims %[[box_load]], %c0{{.*}} : (!fir.box<!fir.heap<!fir.array<?xf64>>>, index) -> (index, index, index)
   ! CHECK: %[[cast:.*]] = fir.convert %[[dim]]#1 : (index) -> i64
-  ! CHECK: fir.call @_FortranAioOutputInteger64(%{{.*}}, %[[cast]]) : (!fir.ref<i8>, i64) -> i1
+  ! CHECK: %[[cast_mul1:.*]] = arith.muli %{{.+}}, %[[cast]] : i64
+  ! CHECK: fir.call @_FortranAioOutputInteger64(%{{.*}}, %[[cast_mul1]]) : (!fir.ref<i8>, i64) -> i1
   print *, size(a, kind=8)
 end subroutine

flang/test/Semantics/modfile17.f90

 !integer(8),kind::k8
 !integer(int(int(k1,kind=1),kind=8))::j1
 !integer(int(int(k2,kind=2),kind=8))::j2
 !integer(int(int(k4,kind=4),kind=8))::j4
 !integer(k8)::j8
 !end type
 !type::defaulted(n1,n2,n4,n8)
 !integer(1),kind::n1=1_1
-!integer(2),kind::n2=int(2_4*int(int(n1,kind=1),kind=4),kind=2)
+!integer(2),kind::n2=int(int(int(n1,kind=1),kind=4)*2_4,kind=2)
 !integer(4),kind::n4=2_4*int(int(n2,kind=2),kind=4)
 !integer(8),kind::n8=int(12_4-int(n4,kind=4),kind=8)
 !type(capture(k1=int(n1,kind=1),k2=int(n2,kind=2),k4=int(n4,kind=4),k8=n8))::cap
 !end type
 !type,extends(defaulted)::extension(k5)
 !integer(4),kind::k5=4_4
 !integer(int(int(k5,kind=4),kind=8))::j5
 !end type

flang/test/Semantics/modfile30.f90

contains
   end
 end
 
 !Expect: m1.mod
 !module m1
 !contains
 ! function f1(x) result(y)
 !  integer(4)::x(:)
-!  integer(4)::y(1_8:size(x,dim=1,kind=8))
+!  integer(4)::y(1_8:int(int(1_8*size(x,dim=1,kind=8),kind=4),kind=8))
 ! end
 ! function f2(x)
 !  integer(4)::x(:)
-!  integer(4)::f2(1_8:size(x,dim=1,kind=8))
+!  integer(4)::f2(1_8:int(int(1_8*size(x,dim=1,kind=8),kind=4),kind=8))
 ! end
 !end
 
 ! Order of names in PUBLIC statement shouldn't affect .mod file.
 module m2
   public :: a
   type t
   end type