[flang] Make folding of LEN less aggressive

[schweitz]

Summary

Folding in the front end was replacing calls to LEN and dropping
arguments even in situations where the arguments are required to compute
the LEN value at runtime.

Add tests.

[PeteSteinfeld]

These changes look good, but when I get errors in ld when I try to build. I'm building on a Linux x86 system using the GNU 9.3.0 toolset. Here's the relevant excerpts from my log file:

[5717/5766] Linking CXX executable tools/flang/unittests/Evaluate/intrinsics.test
FAILED: tools/flang/unittests/Evaluate/intrinsics.test 
: && /usr/bin/c++ -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -Wmisleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -Werror -Wno-deprecated-copy -fno-strict-aliasing -fno-semantic-interposition -O3 -DNDEBUG -Wl,-rpath,/home/sw/thirdparty/gcc/gcc-9.3.0/linux86-64/lib64:/usr/local/lib:/opt/intel/compilers_and_libraries_2020.1.217/linux/compiler/lib/intel64_lin:/opt/intel/compilers_and_libraries_2020.1.217/linux/linux/mpi/intel64/libfabric/lib:/opt/intel/compilers_and_libraries_2020.1.217/linux/linux/mpi/intel64/lib/release:/opt/intel/compilers_and_libraries_2020.1.217/linux/linux/mpi/intel64/lib:/opt/intel/compilers_and_libraries_2020.1.217/linux/ipp/lib/intel64:/opt/intel/compilers_and_libraries_2020.1.217/linux/mkl/lib/intel64_lin:/opt/intel/compilers_and_libraries_2020.1.217/linux/daal/lib/intel64_lin:/opt/intel/compilers_and_libraries_2020.1.217/linux/tbb/lib/intel64/gcc4.8:/opt/intel/debugger_2020/python/intel64/lib:/opt/intel/debugger_2020/libipt/intel64/lib:/home/sw/envmod/modules/linux86-64/latest/lib:  tools/flang/unittests/Evaluate/CMakeFiles/intrinsics.test.dir/intrinsics.cpp.o -o tools/flang/unittests/Evaluate/intrinsics.test  lib/libLLVMSupport.a  lib/libFortranCommon.a  lib/libFortranEvaluateTesting.a  lib/libFortranEvaluate.a  lib/libFortranDecimal.a  lib/libFortranSemantics.a  lib/libFortranParser.a  lib/libFortranRuntime.a  lib/libFortranEvaluate.a  lib/libFortranParser.a  lib/libFortranCommon.a  lib/libLLVMFrontendOpenMP.a  lib/libLLVMScalarOpts.a  lib/libLLVMAggressiveInstCombine.a  lib/libLLVMInstCombine.a  lib/libLLVMTransformUtils.a  lib/libLLVMAnalysis.a  lib/libLLVMProfileData.a  lib/libLLVMSymbolize.a  lib/libLLVMDebugInfoPDB.a  lib/libLLVMDebugInfoMSF.a  lib/libLLVMDebugInfoDWARF.a  lib/libLLVMObject.a  lib/libLLVMBitReader.a  lib/libLLVMCore.a  lib/libLLVMRemarks.a  lib/libLLVMBitstreamReader.a  lib/libLLVMMCParser.a  lib/libLLVMMC.a  lib/libLLVMDebugInfoCodeView.a  lib/libLLVMTextAPI.a  lib/libLLVMBinaryFormat.a  lib/libLLVMFrontendOpenACC.a  lib/libLLVMSupport.a  -lrt  -ldl  -lpthread  -lm  /usr/lib64/libz.so  /usr/lib64/libtinfo.so  lib/libLLVMDemangle.a  lib/libFortranDecimal.a && :
lib/libFortranEvaluate.a(fold-integer.cpp.o): In function `auto Fortran::evaluate::FoldIntrinsicFunction<8>(Fortran::evaluate::FoldingContext&, Fortran::evaluate::FunctionRef<Fortran::evaluate::Type<(Fortran::common::TypeCategory)0, 8> >&&)::{lambda(auto:1&)#14}::operator()<Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)3, 4> > >(Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)3, 4> >&) const':
fold-integer.cpp:(.text._ZZN7Fortran8evaluate21FoldIntrinsicFunctionILi8EEENS0_4ExprINS0_4TypeILNS_6common12TypeCategoryE0EXT_EEEEERNS0_14FoldingContextEONS0_11FunctionRefIS6_EEENKUlRT_E12_clINS2_INS3_ILS5_3ELi4EEEEEEEDaSE_[_ZZN7Fortran8evaluate21FoldIntrinsicFunctionILi8EEENS0_4ExprINS0_4TypeILNS_6common12TypeCategoryE0EXT_EEEEERNS0_14FoldingContextEONS0_11FunctionRefIS6_EEENKUlRT_E12_clINS2_INS3_ILS5_3ELi4EEEEEEEDaSE_]+0x34): undefined reference to `bool Fortran::evaluate::IsConstantExpr<std::optional<Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)0, 8> > > >(std::optional<Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)0, 8> > > const&)'
lib/libFortranEvaluate.a(fold-integer.cpp.o): In function `auto Fortran::evaluate::FoldIntrinsicFunction<8>(Fortran::evaluate::FoldingContext&, Fortran::evaluate::FunctionRef<Fortran::evaluate::Type<(Fortran::common::TypeCategory)0, 8> >&&)::{lambda(auto:1&)#14}::operator()<Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)3, 2> > >(Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)3, 2> >&) const':
fold-integer.cpp:(.text._ZZN7Fortran8evaluate21FoldIntrinsicFunctionILi8EEENS0_4ExprINS0_4TypeILNS_6common12TypeCategoryE0EXT_EEEEERNS0_14FoldingContextEONS0_11FunctionRefIS6_EEENKUlRT_E12_clINS2_INS3_ILS5_3ELi2EEEEEEEDaSE_[_ZZN7Fortran8evaluate21FoldIntrinsicFunctionILi8EEENS0_4ExprINS0_4TypeILNS_6common12TypeCategoryE0EXT_EEEEERNS0_14FoldingContextEONS0_11FunctionRefIS6_EEENKUlRT_E12_clINS2_INS3_ILS5_3ELi2EEEEEEEDaSE_]+0x34): undefined reference to `bool Fortran::evaluate::IsConstantExpr<std::optional<Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)0, 8> > > >(std::optional<Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)0, 8> > > const&)'
lib/libFortranEvaluate.a(fold-integer.cpp.o): In function `auto Fortran::evaluate::FoldIntrinsicFunction<8>(Fortran::evaluate::FoldingContext&, Fortran::evaluate::FunctionRef<Fortran::evaluate::Type<(Fortran::common::TypeCategory)0, 8> >&&)::{lambda(auto:1&)#14}::operator()<Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)3, 1> > >(Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)3, 1> >&) const':
fold-integer.cpp:(.text._ZZN7Fortran8evaluate21FoldIntrinsicFunctionILi8EEENS0_4ExprINS0_4TypeILNS_6common12TypeCategoryE0EXT_EEEEERNS0_14FoldingContextEONS0_11FunctionRefIS6_EEENKUlRT_E12_clINS2_INS3_ILS5_3ELi1EEEEEEEDaSE_[_ZZN7Fortran8evaluate21FoldIntrinsicFunctionILi8EEENS0_4ExprINS0_4TypeILNS_6common12TypeCategoryE0EXT_EEEEERNS0_14FoldingContextEONS0_11FunctionRefIS6_EEENKUlRT_E12_clINS2_INS3_ILS5_3ELi1EEEEEEEDaSE_]+0x34): undefined reference to `bool Fortran::evaluate::IsConstantExpr<std::optional<Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)0, 8> > > >(std::optional<Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)0, 8> > > const&)'
lib/libFortranEvaluate.a(fold-integer.cpp.o): In function `auto Fortran::evaluate::FoldIntrinsicFunction<1>(Fortran::evaluate::FoldingContext&, Fortran::evaluate::FunctionRef<Fortran::evaluate::Type<(Fortran::common::TypeCategory)0, 1> >&&)::{lambda(auto:1&)#14}::operator()<Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)3, 4> > >(Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)3, 4> >&) const':
fold-integer.cpp:(.text._ZZN7Fortran8evaluate21FoldIntrinsicFunctionILi1EEENS0_4ExprINS0_4TypeILNS_6common12TypeCategoryE0EXT_EEEEERNS0_14FoldingContextEONS0_11FunctionRefIS6_EEENKUlRT_E12_clINS2_INS3_ILS5_3ELi4EEEEEEEDaSE_[_ZZN7Fortran8evaluate21FoldIntrinsicFunctionILi1EEENS0_4ExprINS0_4TypeILNS_6common12TypeCategoryE0EXT_EEEEERNS0_14FoldingContextEONS0_11FunctionRefIS6_EEENKUlRT_E12_clINS2_INS3_ILS5_3ELi4EEEEEEEDaSE_]+0x34): undefined reference to `bool Fortran::evaluate::IsConstantExpr<std::optional<Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)0, 8> > > >(std::optional<Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)0, 8> > > const&)'
lib/libFortranEvaluate.a(fold-integer.cpp.o): In function `auto Fortran::evaluate::FoldIntrinsicFunction<1>(Fortran::evaluate::FoldingContext&, Fortran::evaluate::FunctionRef<Fortran::evaluate::Type<(Fortran::common::TypeCategory)0, 1> >&&)::{lambda(auto:1&)#14}::operator()<Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)3, 2> > >(Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)3, 2> >&) const':
fold-integer.cpp:(.text._ZZN7Fortran8evaluate21FoldIntrinsicFunctionILi1EEENS0_4ExprINS0_4TypeILNS_6common12TypeCategoryE0EXT_EEEEERNS0_14FoldingContextEONS0_11FunctionRefIS6_EEENKUlRT_E12_clINS2_INS3_ILS5_3ELi2EEEEEEEDaSE_[_ZZN7Fortran8evaluate21FoldIntrinsicFunctionILi1EEENS0_4ExprINS0_4TypeILNS_6common12TypeCategoryE0EXT_EEEEERNS0_14FoldingContextEONS0_11FunctionRefIS6_EEENKUlRT_E12_clINS2_INS3_ILS5_3ELi2EEEEEEEDaSE_]+0x34): undefined reference to `bool Fortran::evaluate::IsConstantExpr<std::optional<Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)0, 8> > > >(std::optional<Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)0, 8> > > const&)'
lib/libFortranEvaluate.a(fold-integer.cpp.o):fold-integer.cpp:(.text._ZZN7Fortran8evaluate21FoldIntrinsicFunctionILi1EEENS0_4ExprINS0_4TypeILNS_6common12TypeCategoryE0EXT_EEEEERNS0_14FoldingContextEONS0_11FunctionRefIS6_EEENKUlRT_E12_clINS2_INS3_ILS5_3ELi1EEEEEEEDaSE_[_ZZN7Fortran8evaluate21FoldIntrinsicFunctionILi1EEENS0_4ExprINS0_4TypeILNS_6common12TypeCategoryE0EXT_EEEEERNS0_14FoldingContextEONS0_11FunctionRefIS6_EEENKUlRT_E12_clINS2_INS3_ILS5_3ELi1EEEEEEEDaSE_]+0x34): more undefined references to `bool Fortran::evaluate::IsConstantExpr<std::optional<Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)0, 8> > > >(std::optional<Fortran::evaluate::Expr<Fortran::evaluate::Type<(Fortran::common::TypeCategory)0, 8> > > const&)' follow
collect2: error: ld returned 1 exit status
[5718/5766] Linking CXX executable tools/flang/unittests/Evaluate/expression.test
FAILED: tools/flang/unittests/Evaluate/expression.test

[klausler]

flang/lib/Evaluate/fold-integer.cpp

680

IsConstantExpr() is too strict of a test; IsScopeInvariantExpr() would be better.

[PeteSteinfeld]

All builds and tests correctly and looks good.

[jeanPerier]

IsConstantExpr() is too strict of a test; IsScopeInvariantExpr() would be better.

@klausler, while I agree IsConstantExpr is throwing away valid rewrites, IsScopeInvariantExpr is still letting invalid rewrites like the following:

module m
contains
  function cfoo(l)
    integer :: l
    character(l) :: cfoo
    cfoo = "hello"
  end function

  subroutine bug(n)
    integer :: n
    print*, len(cfoo(n))
  end subroutine
end module

flang-new -fc1 -fdebug-unparse % rewrites print*, len(cfoo(n)) in bug into PRINT *, int(cfoo%len,kind=4), that looks wrong to me. And if the l dummy argument is made intent(in) in cfoo, then print*, len(cfoo(n)) is rewritten to PRINT *, int(max(0_8,int(l,kind=8)),kind=4), which is also wrong. The information about the actual argument that defines the length was lost and cannot be retrieved anymore.

So a deeper analysis needs to be done here to avoid using IsConstantExpr, or LEN() should be changed.

flang/lib/Evaluate/fold-integer.cpp

if (name == "abs") { // incl. babs, iiabs, jiaabs, & kiabs
     } else {
       DIE("leadz argument must be integer");
     }
   } else if (name == "len") {
     if (auto *charExpr{UnwrapExpr<Expr<SomeCharacter>>(args[0])}) {
       return std::visit(
           [&](auto &kx) {
             if (auto len{kx.LEN()}) {
+              if (IsConstantExpr(len)) {
                 return Fold(context, ConvertToType<T>(*std::move(len)));
               } else {
                 return Expr<T>{std::move(funcRef)};
               }
+            } else {
+              return Expr<T>{std::move(funcRef)};
+            }
           },
           charExpr->u);
     } else {
       DIE("len() argument must be of character type");
     }
   } else if (name == "len_trim") {
     if (auto *charExpr{UnwrapExpr<Expr<SomeCharacter>>(args[0])}) {
       return std::visit(

flang/test/Evaluate/rewrite01.f90

subroutine lbound_test(x, n, m)
   print *, lbound(returns_array(n, m))
   !CHECK: PRINT *, 1_4
   print *, lbound(returns_array(n, m), dim=1)
   !CHECK: PRINT *, 1_4
   print *, lbound(returns_array_2(m), dim=1)
   !CHECK: PRINT *, 1_4
   print *, lbound(returns_array_3(), dim=1)
 end subroutine
+
+subroutine len_test(a,b, c, d)
+  character(*) :: a, b
+  external b
+  character(10) :: c, d
+  external d
+
+  !CHECK: PRINT *, len(a)
+  print *, len(a)
+  !CHECK: PRINT *, 5_4
+  print *, len(a(1:5))
+  !CHECK: PRINT *, len(b(a))
+  print *, len(b(a))
+  !CHECK: PRINT *, len(b(a)//a)
+  print *, len(b(a) // a)
+  !CHECK: PRINT *, 10_4
+  print *, len(c)
+  !CHECK: PRINT *, len(c(int(i,kind=8):int(j,kind=8)))
+  print *, len(c(i:j))
+  !CHECK: PRINT *, 5_4
+  print *, len(c(1:5))
+  !CHECK: PRINT *, 10_4
+  print *, len(d(c))
+  !CHECK: PRINT *, 20_4
+  print *, len(d(c) // c)
+  !CHECK: PRINT *, 0_4
+  print *, len(a(10:4))
+end subroutine len_test
+
 end module