[flang] Make 'team_number()' an intrinsic function

[craig.rasmussen]

Summary

Added 'team_number()' to the list of functions that are evaluated as intrinsic. Also changed the type of TEAM_TYPE (in TypePattern) to be DerivedType rather than IntType.

Implementation now no longer needs the static function MissingDerivedType() hack.

[craig.rasmussen]

flang/lib/Evaluate/intrinsics.cpp

107 ↗

(On Diff #339011)

I think that the type here should be DerivedType rather than IntType (as I've changed it to be). However, I'm not sure as I'm rather confused about the implications of this. It does apparently fix the intrinsic function image_status() (for which there isn't a test yet).

747 ↗

(On Diff #339011)

I tried to create only one entry here for team_number but couldn't work it out. It seems like Optionality::optional should work but it failed for me (see RequiredTeam and associated comments in the diff above).

1297 ↗

(On Diff #339011)

It seems like more work is required here because argOk==true should only occur if the derived type is type(team_type), right? Can derived types have kinds (I suppose I should know that)?

In any case, once TEAM_TYPE is sorted out I can finish a lot more of coarray intrinsic functions.

[klausler]

flang/lib/Evaluate/intrinsics.cpp

236 ↗

(On Diff #339011)

Why not use it?

1297 ↗

(On Diff #339011)

Right, you need to check that the derived type is TEAM_TYPE from the intrinsic module ISO_FORTRAN_ENV. Grep the sources and you'll find a utility predicate that should help.

[craig.rasmussen]

1. Fixed team argument in call to team_number so that it is optional.
2. Fixed teamType kind in flang/lib/Evaluate/intrinsics.cpp
3. Added team_number test to flang/unittests/Evaluate/intrinsics.cpp

[craig.rasmussen]

flang/include/flang/Common/default-kinds.h

59 ↗

(On Diff #341250)

This was a guess.

flang/unittests/Evaluate/intrinsics.cpp

159

Team is for using a team actual argument in TestCall below at line 323, I don't know how to work this out.

321

Should be fixed or removed I assume.

[klausler]

flang/include/flang/Common/default-kinds.h

59 ↗

(On Diff #341250)

The kind type parameters of derived types are entirely under the control of the program(mer); there can be an arbitrary number of kind type parameters, including zero, and there are no meaningful defaults apart from what a given type's definition may specify.

flang/include/flang/Evaluate/type.h

73 ↗

(On Diff #341250)

Derived types are not intrinsic types; this distinction is their reason for being. Please don't break this predicate.

flang/lib/Common/default-kinds.cpp

79 ↗

(On Diff #341250)

See above. There can be no meaningful default kind that applies to all derived types.

[craig.rasmussen]

I forgot to add that there is one test failing at the moment which makes no sense to me.

FAIL: flang-OldUnit :: Evaluate/folding.test (714 of 799)

• TEST 'flang-OldUnit :: Evaluate/folding.test' FAILED ****

/Users/rasmussen17/Compilers/llvm-project/flang/unittests/Evaluate/folding.cpp:64: FAIL: y1Flushing.IsInfinite() || std::abs(host::CastFortranToHost<R4>(y1Flushing) + 88.) > 2
73 tests pass, 1 test FAILS

---

---

Failed Tests (1):

flang-OldUnit :: Evaluate/folding.test

[tskeith]

In D100897#2723303, @craig.rasmussen wrote:

I forgot to add that there is one test failing at the moment which makes no sense to me.

FAIL: flang-OldUnit :: Evaluate/folding.test (714 of 799)

I assume you're running on a mac. I've seen that too but haven't been able to figure out what the problem is. It's not related to your change so you can ignore it.

[craig.rasmussen]

Added the static function MissingDerivedType() to allow the creation of a DynamicType with TypeCategory::Derived when a semantics::DerivedTypeSpec is not directly available. This allows a missing actual argument for an optional dummy.

[craig.rasmussen]

Added line comments.

flang/include/flang/Evaluate/type.h

132 ↗

(On Diff #357042)

This function (whatever it is named) is critical to get team_number to work with a missing optional team arguments. Otherwise the check

CHECK(IsValidKindOfIntrinsicType(category_, kind_));

disallows the construction of a DynamicType that is not an intrinsic type.

226 ↗

(On Diff #357042)

An special enum value is not really needed (I'm pretty sure) as a kind value of 0 could be used. Having a special kind value could be useful for later analysis but no usage identified at the moment.

flang/lib/Evaluate/intrinsics.cpp

1688 ↗

(On Diff #357042)

Since there is no actual argument at this point a semantics::DerivedTypeSpec can't be created from it. Nor does it seem possible to create a DerivedTypeSpec from the context as it is a FoldingContext not a semantics::SemanticsContext.

[klausler]

flang/include/flang/Evaluate/type.h

132 ↗

(On Diff #357042)

When the derived type TEAM_TYPE from intrinsic module ISO_FORTRAN_ENV is available, this MissingDerivedType() is not required, right?

[craig.rasmussen]

flang/include/flang/Evaluate/type.h

132 ↗

(On Diff #357042)

Correct. This is tested by the new test team_number.f90 (added in this patch). If the actual argument is available the dummy argument is created from the actual with

auto dc{characteristics::DummyArgument::FromActual(
    std::string{d.keyword}, *expr, context)};

see semantics.cpp line 1651. Although to be clear, the derived type TEAM_TYPE will have already been loaded from ISO_FORTRAN_ENV via a USE statement by this time, it is just not directly available.

[klausler]

flang/include/flang/Evaluate/type.h

132 ↗

(On Diff #357042)

So when the optional argument is absent, the type is *not* TEAM_TYPE from ISO_FORTRAN_ENV, even if that type is available?

[craig.rasmussen]

flang/include/flang/Evaluate/type.h

132 ↗

(On Diff #357042)

I'm not sure sure I understand the question fully but I'll give it a try. If the actual argument is missing, the dummy argument type, TEAM_TYPE, remains the same. However, one can't create a characteristics::DummyArgument from the actual argument (which happens if the actual is present) as it is missing. Thus I created the new function MissingDerivedType() to create a DynamicType.

Admittedly, the DynamicType is not well formed because the derived_ member is a nullptr. However, is the semantic checking that follows this does not create a problem because when the actual argument is missing, there is not type comparison done between the actual argument (since it is missing) and the dummy argument.

If I understand correctly, I don't think the AST is mutated during this pass the DynamicType returned by MissingDerivedType() is not ultimately kept around.

[craig.rasmussen]

1. Added "! REQUIRES: shell" to test file so as not to trip on windows
2. Ran git clang-format

[craig.rasmussen]

@klausler, are any further changes required to get this approved?

[klausler]

In D100897#2982831, @craig.rasmussen wrote:

@klausler, are any further changes required to get this approved?

Have you given up trying to construct a valid TEAM_TYPE derived type to characterize an absent TEAM= actual argument?

[craig.rasmussen]

In a response @klausler asked:

Have you given up trying to construct a valid TEAM_TYPE derived type to characterize an absent TEAM= actual argument?

Yes. When I looked into doing this it didn't seem possible without major changes to the function interfaces in lib/Evaluate/intrinsics.cpp. As far as I can tell, it doesn't seem possible to create a DerivedTypeSpec from the context, as it is a FoldingContext not a semantics::SemanticsContext.

[klausler]

In D100897#2992779, @craig.rasmussen wrote:

In a response @klausler asked:

Have you given up trying to construct a valid TEAM_TYPE derived type to characterize an absent TEAM= actual argument?

Yes. When I looked into doing this it didn't seem possible without major changes to the function interfaces in lib/Evaluate/intrinsics.cpp. As far as I can tell, it doesn't seem possible to create a DerivedTypeSpec from the context, as it is a FoldingContext not a semantics::SemanticsContext.

I'll have to do it, then.

[PeteSteinfeld]

In D100897#2992783, @klausler wrote:

In D100897#2992779, @craig.rasmussen wrote:

In a response @klausler asked:

Have you given up trying to construct a valid TEAM_TYPE derived type to characterize an absent TEAM= actual argument?

Yes. When I looked into doing this it didn't seem possible without major changes to the function interfaces in lib/Evaluate/intrinsics.cpp. As far as I can tell, it doesn't seem possible to create a DerivedTypeSpec from the context, as it is a FoldingContext not a semantics::SemanticsContext.

I'll have to do it, then.

Would it be possible to create a constant of type TEAM_TYPE in iso_fortran_env.f90 or __fortran_builtins.f90 to indicate the current team and then use that constant as the value of the missing optional argument? This would mean that iso_fortran_env.f90 would need to be available to the compiler to process calls to the TEAM_NUMBER() intrinsic.

[klausler]

Would it be possible to create a constant of type TEAM_TYPE in iso_fortran_env.f90 or __fortran_builtins.f90 to indicate the current team and then use that constant as the value of the missing optional argument? This would mean that iso_fortran_env.f90 would need to be available to the compiler to process calls to the TEAM_NUMBER() intrinsic.

Sure, but finding that constant is no easier than finding TEAM_TYPE itself. Not a big deal, and I'll look into it when I'm out from under the f95 semantics items.

[craig.rasmussen]

Implemented TEAM_NUMBER intrinsic function without use of MissingDerivedType() hack.

[klausler]

See https://reviews.llvm.org/D110356.

[craig.rasmussen]

flang/lib/Evaluate/intrinsics.cpp

1297 ↗

(On Diff #339011)

I had found semantics::IsTeamType which worked fine when static libraries are built, however fails when building shared libraries. Spent too much time trying to figure out a way around the linker but its pretty clear that the design doesn't really allow this because of library build order. Now just using a simple lookup for symbol name "__builtin_team_type". Will submit new patch shortly after more testing,

[klausler]

flang/lib/Evaluate/intrinsics.cpp

1297 ↗

(On Diff #339011)

Please see https://reviews.llvm.org/D110356.

[craig.rasmussen]

Inlined implementation of call to semantics::IsTeamType. This is necessary because the semantics::IsTeamType symbol in undefined at the time libFortranEvalute is built.

[craig.rasmussen]

flang/lib/Evaluate/intrinsics.cpp

1297 ↗

(On Diff #339011)

Ok, just noticed your comment. I'll update the patch.

[craig.rasmussen]

This needs to be updated for changes in https://reviews.llvm.org/D110356

[craig.rasmussen]

Update based on changes in https://reviews.llvm.org/D110356. Cleaned up tests.

[clementval]

Looks like buildbots are failing with this revision. Can you have a look or revert?

[DavidSpickett]

The failure we have is: https://lab.llvm.org/buildbot/#/builders/176/builds/951/steps/12/logs/FAIL__flang-OldUnit__intrinsics_test

INTERNAL: The __fortran_builtins module was not found, and the type '__builtin_team_type' was required.

[rovka]

Is this still failing after
https://github.com/llvm/llvm-project/commit/18e63289358f50143fe4bc59942f3959bdffa772#diff-bc82e30d81709b26f2c8030300784e95003f1b968e1ed7842fa717858e497336
?

[DavidSpickett]

Is isn't, https://lab.llvm.org/buildbot/#/builders/177/builds/1621.

False alarm then, thanks! I wondered why the flang bots were being very slow today, turns out they aren't at all.

flang/docs/Intrinsics.md

 
 ### Semantic Analysis
 F18 semantic expression analysis phase detects intrinsic procedure references,
 validates the argument types and deduces the return types.
 This phase currently supports all the intrinsic procedures listed above but the ones in the table below.
 
 | Intrinsic Category | Intrinsic Procedures Lacking Support |
 | --- | --- |
-| Coarray intrinsic functions | LCOBOUND, UCOBOUND, FAILED_IMAGES, GET_TEAM, IMAGE_INDEX, STOPPED_IMAGES, TEAM_NUMBER, COSHAPE |
+| Coarray intrinsic functions | LCOBOUND, UCOBOUND, FAILED_IMAGES, IMAGE_INDEX, STOPPED_IMAGES, COSHAPE |
 | Object characteristic inquiry functions | ALLOCATED, ASSOCIATED, EXTENDS_TYPE_OF, IS_CONTIGUOUS, PRESENT, RANK, SAME_TYPE, STORAGE_SIZE |
 | Type inquiry intrinsic functions | BIT_SIZE, DIGITS, EPSILON, HUGE, KIND, MAXEXPONENT, MINEXPONENT, NEW_LINE, PRECISION, RADIX, RANGE, TINY|
 | Non-standard intrinsic functions | AND, OR, XOR, LSHIFT, RSHIFT, SHIFT, ZEXT, IZEXT, COSD, SIND, TAND, ACOSD, ASIND, ATAND, ATAN2D, COMPL, DCMPLX, EQV, NEQV, INT8, JINT, JNINT, KNINT, LOC, QCMPLX, DREAL, DFLOAT, QEXT, QFLOAT, QREAL, DNUM, NUM, JNUM, KNUM, QNUM, RNUM, RAN, RANF, ILEN, SIZEOF, MCLOCK, SECNDS, COTAN, IBCHNG, ISHA, ISHC, ISHL, IXOR, IARG, IARGC, NARGS, NUMARG, BADDRESS, IADDR, CACHESIZE, EOF, FP_CLASS, INT_PTR_KIND, ISNAN, MALLOC |
 | Intrinsic subroutines |MVBITS (elemental), CPU_TIME, DATE_AND_TIME, EVENT_QUERY, EXECUTE_COMMAND_LINE, GET_COMMAND, GET_COMMAND_ARGUMENT, GET_ENVIRONMENT_VARIABLE, MOVE_ALLOC, RANDOM_INIT, RANDOM_NUMBER, RANDOM_SEED, SYSTEM_CLOCK |
 | Atomic intrinsic subroutines | ATOMIC_ADD &al. |
 | Collective intrinsic subroutines | CO_BROADCAST &al. |
 
 

flang/test/Semantics/team_number.f90

+! RUN: %python %S/test_errors.py %s %flang_fc1
+! Check for semantic errors in team_number() function calls
+
+subroutine test
+  use, intrinsic :: iso_fortran_env, only: team_type
+  type(team_type) :: team
+
+  ! correct calls, should produce no errors
+  team = get_team()
+  print *, team_number()
+  print *, team_number(team)
+  print *, team_number(team=team)
+
+  ! call with too many arguments
+  !ERROR: too many actual arguments for intrinsic 'team_number'
+  print *, team_number(1, 3)
+
+  ! keyword argument with incorrect type
+  !ERROR: Actual argument for 'team=' has bad type 'REAL(4)'
+  print *, team_number(team=3.1415)
+
+end subroutine

flang/unittests/Evaluate/intrinsics.cpp

void TestIntrinsics() {
   using Int4 = Type<TypeCategory::Integer, 4>;
   using Int8 = Type<TypeCategory::Integer, 8>;
   using Real4 = Type<TypeCategory::Real, 4>;
   using Real8 = Type<TypeCategory::Real, 8>;
   using Complex4 = Type<TypeCategory::Complex, 4>;
   using Complex8 = Type<TypeCategory::Complex, 8>;
   using Char = Type<TypeCategory::Character, 1>;
   using Log4 = Type<TypeCategory::Logical, 4>;
 
   TestCall{defaults, table, "bad"}
       .Push(Const(Scalar<Int4>{}))
       .DoCall(); // bad intrinsic name
   TestCall{defaults, table, "abs"}
       .Push(Named("a", Const(Scalar<Int4>{})))
       .DoCall(Int4::GetType());
   TestCall{defaults, table, "abs"}
       .Push(Const(Scalar<Int4>{}))
TestCall{defaults, table, "num_images"}
       .DoCall(); // bad type
   TestCall{defaults, table, "num_images"}
       .Push(Const(Scalar<Complex8>{}))
       .DoCall(); // bad type
   TestCall{defaults, table, "num_images"}
       .Push(Const(Scalar<Real4>{}))
       .DoCall(); // bad type
 
+  TestCall{defaults, table, "team_number"}.DoCall(Int4::GetType());
+  TestCall{defaults, table, "team_number"}
+      .Push(Const(Scalar<Int4>{}))
+      .Push(Const(Scalar<Int4>{}))
+      .DoCall(); // too many args
+  TestCall{defaults, table, "team_number"}
+      .Push(Named("bad", Const(Scalar<Int4>{})))
+      .DoCall(); // bad keyword
+  TestCall{defaults, table, "team_number"}
+      .Push(Const(Scalar<Int4>{}))
+      .DoCall(); // bad type
+  TestCall{defaults, table, "team_number"}
+      .Push(Const(Scalar<Char>{}))
+      .DoCall(); // bad type
+  TestCall{defaults, table, "team_number"}
+      .Push(Const(Scalar<Log4>{}))
+      .DoCall(); // bad type
+  TestCall{defaults, table, "team_number"}
+      .Push(Const(Scalar<Complex8>{}))
+      .DoCall(); // bad type
+  TestCall{defaults, table, "team_number"}
+      .Push(Const(Scalar<Real4>{}))
+      .DoCall(); // bad type
+
   // TODO: test other intrinsics
 
   // Test unrestricted specific to generic name mapping (table 16.2).
   TEST(table.GetGenericIntrinsicName("alog") == "log");
   TEST(table.GetGenericIntrinsicName("alog10") == "log10");
   TEST(table.GetGenericIntrinsicName("amod") == "mod");
   TEST(table.GetGenericIntrinsicName("cabs") == "abs");
   TEST(table.GetGenericIntrinsicName("ccos") == "cos");
   TEST(table.GetGenericIntrinsicName("cexp") == "exp");