This is an archive of the discontinued LLVM Phabricator instance.

[flang] Increase support for intrinsic module procedures
ClosedPublic

Authored by clementval on Jun 23 2022, 5:16 AM.

Details

Summary
  • Make Semantics test doconcurrent01.f90 an expected failure pending a fix

for a problem in recognizing a PURE prefix specifier for a specific procedure
that occurs in new intrinsic module source code,

  • review update
  • review update
  • Increase support for intrinsic module procedures

The f18 standard defines 5 intrinsic modules that define varying numbers
of procedures, including several operators:

2  iso_fortran_env

55 ieee_arithmetic
10 ieee_exceptions

0  ieee_features
6  iso_c_binding

There are existing fortran source files for each of these intrinsic modules.
This PR adds generic procedure declarations to these files for procedures
that do not already have them, together with associated specific procedure
declarations. It also adds the capability of recognizing intrinsic module
procedures in lowering code, making it possible to use existing language
intrinsic code generation for intrinsic module procedures for both scalar
and elemental calls. Code can then be generated for intrinsic module
procedures using existing options, including front end folding, direct
inlining, and calls to runtime support routines. Detailed code generation
is provided for several procedures in this PR, with others left to future PRs.
Procedure calls that reach lowering and don't have detailed implementation
support will generate a "not yet implemented" message with a recognizable name.

The generic procedures in these modules may each have as many as 36 specific
procedures. Most specific procedures are generated via macros that generate
type specific interface declarations. These specific declarations provide
detailed argument information for each individual procedure call, similar
to what is done via other means for standard language intrinsics. The
modules only provide interface declarations. There are no procedure
definitions, again in keeping with how language intrinsics are processed.

This patch is part of the upstreaming effort from fir-dev branch.

Co-authored-by: V Donaldson <vdonaldson@nvidia.com>

Diff Detail

Event Timeline

clementval created this revision.Jun 23 2022, 5:16 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJun 23 2022, 5:16 AM
Herald added a subscriber: mehdi_amini. · View Herald Transcript
clementval requested review of this revision.Jun 23 2022, 5:16 AM
jeanPerier accepted this revision.Jun 23 2022, 7:08 AM

Looks good, thanks

This revision is now accepted and ready to land.Jun 23 2022, 7:08 AM
PeteSteinfeld accepted this revision.Jun 23 2022, 7:17 AM

All builds and tests correctly and looks good.

This revision was automatically updated to reflect the committed changes.

The code looks good, except that we don't want to revert the changes to the two Fortran module source files. (We want to keep the pure prefixes.)

Otherwise, looks good. Thanks!

flang/module/__fortran_ieee_exceptions.f90
127

The llvm versions of files __fortran_ieee_exceptions.f90 and ieee_arithmetic.f90 are correct. We need to keep the pure prefixes here and below. (They were added after this primary change.) We can just drop these two files from the review.

clementval added inline comments.Jun 24 2022, 12:04 AM
flang/module/__fortran_ieee_exceptions.f90
127
rogfer01 added inline comments.
flang/lib/Lower/IntrinsicCall.cpp
1541

I'm trying to understand the code around this area.

This genericName function is converting (among other cases) things like __builtin_ieee_is_nan into ieee_is_nan which seems reasonable.

However, this function may be called from ScalarExprLowering::genIntrinsicRef in ConvertExpr.cpp which first checks the kind of argument passing required

const Fortran::lower::IntrinsicArgumentLoweringRules *argLowering =  
    Fortran::lower::getIntrinsicArgumentLowering(name);

However, here (this is ConvertExpr.cpp) name will be __builtin_ieee_is_nan. When we query the lowering argument rules we will get an empty result and pass by value. This case probably works fine for ieee_is_nan, but a __builtin_* intrinsic that expects a box (for reasons) will result in a mismatch between its handler defined in IntrinsicCall.cpp and this code.

For instance c_loc, aka __builtin_c_loc, could be (not saying it must be! Hence my question) implemented by passing it a box so BoxAddrOp can be used to extract the address and wrap it in a type(c_ptr). The current module definition ends aliasing it to __builtin_c_loc. So in ConvertExpr.cpp the name used is __builtin_c_loc but in IntrinsicCall.cpp we expect c_loc. Sure, an option is two have the two names (c_loc and __builtin_c_loc) in the handlers[] array of IntrinsicCall.cpp but I think this is not the intent here.

Is the situation I describe an impossible one that should never happen or the intrinsic names should always be adjusted when querying them in the handlers array?