This is an archive of the discontinued LLVM Phabricator instance.

[flang] Make folding of LEN less aggressive
ClosedPublic

Authored by schweitz on Feb 9 2022, 1:27 PM.

Details

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.

Diff Detail

Event Timeline

schweitz created this revision.Feb 9 2022, 1:27 PM
schweitz requested review of this revision.Feb 9 2022, 1:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 9 2022, 1:27 PM
PeteSteinfeld requested changes to this revision.Feb 9 2022, 2:13 PM

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
This revision now requires changes to proceed.Feb 9 2022, 2:13 PM
klausler added inline comments.Feb 9 2022, 2:19 PM
flang/lib/Evaluate/fold-integer.cpp
680

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

schweitz updated this revision to Diff 407332.Feb 9 2022, 4:07 PM
schweitz updated this revision to Diff 407341.Feb 9 2022, 4:31 PM
schweitz updated this revision to Diff 407342.Feb 9 2022, 4:33 PM
PeteSteinfeld accepted this revision.Feb 9 2022, 4:47 PM

All builds and tests correctly and looks good.

This revision is now accepted and ready to land.Feb 9 2022, 4:47 PM
This revision was landed with ongoing or failed builds.Feb 9 2022, 4:50 PM
This revision was automatically updated to reflect the committed changes.

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.