This is an archive of the discontinued LLVM Phabricator instance.

[flang][msvc] Workaround 'forgotten' symbols in FoldOperation. NFC.
ClosedPublic

Authored by Meinersbur on Sep 29 2020, 10:31 AM.

Details

Summary

This resolves an issue where the Microsoft compiler 'forgets' symbols when using constexpr in a lambda in a templated function. The symbols are:

  1. The implicit lambda captures context and convert. Fix by making them explicit captures. The error message was:
fold-implementation.h(1220): error C2065: 'convert': undeclared identifier
  1. The function template argument FROMCAT. Fix by storing it in a temporary constexpr variable inside the function. The error message was:
fold-implementation.h(1216): error C2065: 'FROMCAT': undeclared identifier

This patch is part of the series to make flang compilable with MS Visual Studio http://lists.llvm.org/pipermail/flang-dev/2020-July/000448.html.

Diff Detail

Event Timeline

Meinersbur created this revision.Sep 29 2020, 10:31 AM
Herald added a project: Restricted Project. · View Herald Transcript
Meinersbur requested review of this revision.Sep 29 2020, 10:31 AM

Using the switch /Zc:lambda also allows to compile this code. The switch is not documented in the Visual Studio help, but cl /help documents it as:

/Zc:arg1[,arg2] C++ language conformance, where arguments can be:
  [...]
  lambda[-]             better lambda support by using the newer lambda processor (off by default)
klausler accepted this revision.Sep 29 2020, 12:25 PM

If the magic MSVC option works around this compiler bug, and perhaps others, it seems to me like a cleaner way to avoid their problems.

flang/lib/Evaluate/fold-implementation.h
1160

Please conform to our conventional use of braced initialization, and consider adding a comment that the new constexpr is a MSVC work-around, or it'll be susceptible to being removed as redundant.

This revision is now accepted and ready to land.Sep 29 2020, 12:25 PM

/Zc:lambda fixes D88052 and this compiler bug. This flag was actually suggested by Microsoft in the msvc bug report for D88052.

I created all the reviews to compile flang with msvc (some tests are failing though), these two are the only patches necessary to compile without /Zc:lambda. I am happy to create patch that adds /Zc:lambda when compiling flang if needed, but considering that its state of support is unclear to me, I am not sure whether it is worth it over two smaller workarounds. I would also have to check that whether other problems appear when compiling all of flang with that flag.

Meinersbur retitled this revision from [flang][msvc] Workaround 'forgotten' symbols FoldOperation. NFC. to [flang][msvc] Workaround 'forgotten' symbols in FoldOperation. NFC..Sep 29 2020, 8:38 PM

Indeed, switchin on /Zc:lambda globally results in a different compile failure:

flang\lib\Evaluate\intrinsics-library-templates.h(199,63): error C2664: 'Fortran::evaluate::value::Real<Fortran::evaluate::value::Integer<64,fal
se,32,unsigned int,unsigned __int64>,53> (Fortran::evaluate::FoldingContext &,Fortran::evaluate::GenericFunctionPointer,const Fortran::evaluate::value::Real<Fortran::evaluate::value
::Integer<64,false,32,unsigned int,unsigned __int64>,53> &,const Fortran::evaluate::value::Real<Fortran::evaluate::value::Integer<64,false,32,unsigned int,unsigned __int64>,53> &)':
 cannot convert argument 3 from 'const ConstantContainer<T>' to 'const Fortran::evaluate::value::Real<Fortran::evaluate::value::Integer<64,false,32,unsigned int,unsigned __int64>,53
> &'
          with
          [
              T=Fortran::evaluate::Type<Fortran::common::TypeCategory::Real,8>
          ]
flang\lib\Evaluate\intrinsics-library-templates.h(173,36): message : Reason: cannot convert from 'const ConstantContainer<T>' to 'const Fortran:
:evaluate::value::Real<Fortran::evaluate::value::Integer<64,false,32,unsigned int,unsigned __int64>,53>'
          with
          [
              T=Fortran::evaluate::Type<Fortran::common::TypeCategory::Real,8>
          ]