The intrinsics DREAL, DIMAG, and DCONJG are from Fortran 77 extensions.
For DREAL, the type of argument is extended to any complex. For DIMAG
and DCONJG, the type of argument for them should be complex(8). For DIMAG,
the result type should be real(8). For DCONJG, the result type should be
complex(8). Fix the intrinsic interface for them and add test cases for
the semantic checks and the lowering.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
gfortran 11.2:
https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gfortran/DREAL.html#DREAL
https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gfortran/AIMAG.html#AIMAG
https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gfortran/CONJG.html#CONJG
ifort:
https://www.intel.com/content/www/us/en/develop/documentation/fortran-compiler-oneapi-dev-guide-and-reference/top/language-reference/a-to-z-reference/c-to-d/dreal.html#dreal
https://www.intel.com/content/www/us/en/develop/documentation/fortran-compiler-oneapi-dev-guide-and-reference/top/language-reference/a-to-z-reference/a-to-b/aimag.html#aimag
https://www.intel.com/content/www/us/en/develop/documentation/fortran-compiler-oneapi-dev-guide-and-reference/top/language-reference/a-to-z-reference/c-to-d/conjg.html#conjg
- Update the summary: Fortran 77 -> Fortran 77 extensions.
- Remove the isRestrictedSpecific for DIMAG and DCONJG.
The semantic errors are tested for gfortran 11.2/ifort 2021.5.0/ifx 2022.0.0 in https://gcc.godbolt.org/z/dT1dchYjY. The patch makes it to be consistent with gfortran/ifort/ifx.
Given dconjg and dimag are unrestricted specific intrinsic procedures, your change makes sense to me (we need an exact type for them, and we most likely had issues when lowering code that passed them as dummy procedure arguments).
However, given dreal is restricted, I do not know how useful it is to forbid it from accepting arguments of any complex type. In my opinion, the behavior of accepting this as an extension is pretty clear (as mentioned in the comment updated by this patch). Nvfortran actually accepts non complex(8) arguments. But I grant you it is the only compiler accepting that, so I am not strongly opposed being stricter here too. I would vote to keep dreal accepting non complex(8) so that we are consistent with the behavior of accepting this on all the specific conversion intrinsic procedures (dcmplx, dreal, float, idint, ifix, and sngl).
So please wait for a second opinion on dreal.
flang/test/Lower/intrinsics-1.f90 | ||
---|---|---|
4 ↗ | (On Diff #421772) | Intrinsics lowering tests are in https://github.com/llvm/llvm-project/tree/main/flang/test/Lower/Intrinsics/[intrinsic-name].f90, please use that pattern for these new tests. |
I would vote to keep dreal accepting non complex(8) so that we are consistent with the behavior of accepting this on all the specific conversion intrinsic procedures (dcmplx, dreal, float, idint, ifix, and sngl).
For dcmplx, gfortran and ifort accepts any integer, real or complex. It seems each intrinsic should be checked case-by-case.
https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gfortran/DCMPLX.html#DCMPLX
https://www.intel.com/content/www/us/en/develop/documentation/fortran-compiler-oneapi-dev-guide-and-reference/top/language-reference/a-to-z-reference/c-to-d/dcmplx.html
For dreal, accepting complex(4) is totally fine and should have no risk. For complex(16), one warning may should be reported.
I will wait for others' comments.
Move flang/test/Lower/intrinsics-1.f90 to flang/test/Lower/Intrinsics/[intrinsic-name].f90.
The restricted specific intrinsic could actually be loosened to accept any numeric argument and would still work. I don't see why it should be made more particular. Usage accepted by other compilers should work in f18 unless there's a good reason otherwise.
Thanks @jeanPerier and @klausler for the opinions. Remove the DREAL fix and change the title and summary.
Although I think one warning should be emitted for the argument complex(16), I am not working on this for now. Current LLVM Flang doesn't emit warnings for a variety of cases other compilers support such as passing the actual argument of character(10) to the dummy argument of character(2). I will give high priority for severe problems, especially severe errors in workloads.