Re: Fortran vector math header
On Tue, Feb 05, 2019 at 01:47:57PM +0100, Martin Liška wrote: > > gcc/fortran/ChangeLog: > > 2019-01-24 Martin Liska > > * decl.c (gfc_match_gcc_builtin): Add support for filtering > of builtin directive based on multilib ABI name. > > gcc/testsuite/ChangeLog: > > 2019-01-24 Martin Liska > > * gfortran.dg/simd-builtins-7.f90: New test. > * gfortran.dg/simd-builtins-7.h: New test. The Fortran bits look ok to me. -- steve
Re: Fortran vector math header
On 2/14/19 10:13 PM, Steve Ellcey wrote: > On Wed, 2019-02-13 at 12:34 +0100, Martin Liška wrote: >> May I please ping this so that we can reach mainline soon? >> >> Thanks, >> Martin > > Martin, I can't approve this patch but I can say that I have used it on > Aarch64 and created a follow up patch for aarch64 to create a > get_multilib_abi_name target function for that platform. Everything > seemed to work fine for me and I did not have any problems or see any > regressions when using your patch. Great, can you please send the patch to this email thread? > I hope it gets approved and checked > in soon. Me too :) Martin > > Steve Ellcey > sell...@marvell.com >
Re: Fortran vector math header
On Wed, 2019-02-13 at 12:34 +0100, Martin Liška wrote: > May I please ping this so that we can reach mainline soon? > > Thanks, > Martin Martin, I can't approve this patch but I can say that I have used it on Aarch64 and created a follow up patch for aarch64 to create a get_multilib_abi_name target function for that platform. Everything seemed to work fine for me and I did not have any problems or see any regressions when using your patch. I hope it gets approved and checked in soon. Steve Ellcey sell...@marvell.com
Re: Fortran vector math header
On Tue, 5 Feb 2019, Martin Liška wrote: > #ifdef NATIVE_SYSTEM_HEADER_DIR >/* Then search: /usr/include/finclude/ */ >add_sysrooted_hdrs_prefix (, NATIVE_SYSTEM_HEADER_DIR > "/finclude/", > - NULL, 0, 0, false); > + NULL, 0, 0, 0); > #endif The comment needs updating to reflect that the search no longer uses multilib suffixes. The driver changes are OK with that change. -- Joseph S. Myers jos...@codesourcery.com
Re: Fortran vector math header
May I please ping this so that we can reach mainline soon? Thanks, Martin On 2/5/19 1:48 PM, Martin Liška wrote: > On 2/5/19 2:31 AM, Joseph Myers wrote: >> My main comment here is that if you go with the approach of a single >> header shared by multilibs, you should also update the driver code so it >> no longer uses any sort of multilib suffix when searching for this header >> (it *should* still use the sysroot headers suffix when searching in a >> sysroot). > > Addressed in the reply I've just sent to the Jakub's email. > > Martin >
Re: Fortran vector math header
On 2/5/19 2:31 AM, Joseph Myers wrote: > My main comment here is that if you go with the approach of a single > header shared by multilibs, you should also update the driver code so it > no longer uses any sort of multilib suffix when searching for this header > (it *should* still use the sysroot headers suffix when searching in a > sysroot). Addressed in the reply I've just sent to the Jakub's email. Martin
Re: Fortran vector math header
On 2/4/19 11:10 AM, Jakub Jelinek wrote:
> On Thu, Jan 24, 2019 at 04:25:13PM +0100, Martin Liška wrote:
>> @@ -11361,6 +11365,13 @@ gfc_match_gcc_builtin (void)
>>else if (gfc_match (" ( inbranch ) ") == MATCH_YES)
>> clause = SIMD_INBRANCH;
>>
>> + if (gfc_match (" if ( '%n' ) ", target) == MATCH_YES)
>
> Not sure if it is clean enough to have the 's in there, rather than
> matching there a string literal, requiring that it is a constant and getting
> at the string in there. But if this is fine with the Fortran maintainers, I
> won't object.
>
> Given the patches for aarch64, I'd think that in addition to the target
> specific multilib strings it might be worth to handle here also the generic
> lp64, ilp32 and llp64 strings (by checking the precision of the
> corresponding C types) and perhaps also big-endian, little-endian and
> pdp-endian. This way, targets which have only those differences wouldn't
> need to come up with their hooks.
I would prefer to leave the implementation as simple as possible. I don't expect
many targets implementing the SIMD ABI in glibc. It would not require much work
on glibc side (math header file).
>
>> +! { dg-final { scan-tree-dump "sinf.simdclone" "optimized" { target ilp32 }
>> } } */
>> +! { dg-final { scan-tree-dump-not "sin.simdclone" "optimized" { target
>> ilp32 } } } */
>
> I think you need ia32 here, otherwise it will fail on x32, which is also
> ilp32 target, but not i386.
Fixed.
>
>> +! { dg-final { scan-tree-dump "sin.simdclone" "optimized" { target lp64} }
>> } */
>> +! { dg-final { scan-tree-dump-not "sinf.simdclone" "optimized" { target
>> lp64 } } } */
>> diff --git a/gcc/testsuite/gfortran.dg/simd-builtins-7.h
>> b/gcc/testsuite/gfortran.dg/simd-builtins-7.h
>> new file mode 100644
>> index 000..1c19b88f877
>> --- /dev/null
>> +++ b/gcc/testsuite/gfortran.dg/simd-builtins-7.h
>> @@ -0,0 +1,2 @@
>> +!GCC$ builtin (sin) attributes simd (notinbranch) if('x86_64')
>> +!GCC$ builtin (sinf) attributes simd (notinbranch) if('i386')
>
> That is all from me, but I think you need a buy-in from the Fortran
> maintainers (if they are ok with such an extension from Fortran language
> POV) and from Joseph (or other glibc people).
I CCed Fortran ML, I'm attaching update patch.
Martin
>
> Jakub
>
>From 74fa90f002d862582db7f613ff7aa758a0cbc5c0 Mon Sep 17 00:00:00 2001
From: marxin
Date: Mon, 21 Jan 2019 08:46:06 +0100
Subject: [PATCH] Support if statement in !GCC$ builtin directive.
gcc/ChangeLog:
2019-01-24 Martin Liska
* config/i386/i386.c (ix86_get_multilib_abi_name): New function.
(TARGET_GET_MULTILIB_ABI_NAME): New macro defined.
* doc/tm.texi: Document new target hook.
* doc/tm.texi.in: Likewise.
* target.def: Add new target macro.
* gcc.c (find_fortran_preinclude_file): Do not search multilib
suffixes.
gcc/fortran/ChangeLog:
2019-01-24 Martin Liska
* decl.c (gfc_match_gcc_builtin): Add support for filtering
of builtin directive based on multilib ABI name.
gcc/testsuite/ChangeLog:
2019-01-24 Martin Liska
* gfortran.dg/simd-builtins-7.f90: New test.
* gfortran.dg/simd-builtins-7.h: New test.
---
gcc/config/i386/i386.c| 17 +++
gcc/doc/tm.texi | 4
gcc/doc/tm.texi.in| 2 ++
gcc/fortran/decl.c| 21 ++-
gcc/gcc.c | 10 -
gcc/target.def| 6 ++
gcc/testsuite/gfortran.dg/simd-builtins-7.f90 | 19 +
gcc/testsuite/gfortran.dg/simd-builtins-7.h | 2 ++
8 files changed, 71 insertions(+), 10 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/simd-builtins-7.f90
create mode 100644 gcc/testsuite/gfortran.dg/simd-builtins-7.h
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 789a53501ee..232a14a7de7 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -29583,6 +29583,19 @@ ix86_warn_parameter_passing_abi (cumulative_args_t cum_v, tree type)
cum->warn_empty = false;
}
+/* This hook returns name of multilib ABI. */
+
+static const char *
+ix86_get_multilib_abi_name (void)
+{
+ if (!(TARGET_64BIT_P (ix86_isa_flags)))
+return "i386";
+ else if (TARGET_X32_P (ix86_isa_flags))
+return "x32";
+ else
+return "x86_64";
+}
+
/* Compute the alignment for a variable for Intel MCU psABI. TYPE is
the data type, and ALIGN is the alignment that the object would
ordinarily have. */
@@ -51815,6 +51828,10 @@ ix86_run_selftests (void)
#undef TARGET_WARN_PARAMETER_PASSING_ABI
#define TARGET_WARN_PARAMETER_PASSING_ABI ix86_warn_parameter_passing_abi
+#undef TARGET_GET_MULTILIB_ABI_NAME
+#define TARGET_GET_MULTILIB_ABI_NAME \
+ ix86_get_multilib_abi_name
+
#if CHECKING_P
#undef TARGET_RUN_TARGET_SELFTESTS
#define TARGET_RUN_TARGET_SELFTESTS selftest::ix86_run_selftests
diff --git a/gcc/doc/tm.texi
Re: Fortran vector math header
On Mon, 4 Feb 2019, Jakub Jelinek wrote:
> > +!GCC$ builtin (sin) attributes simd (notinbranch) if('x86_64')
> > +!GCC$ builtin (sinf) attributes simd (notinbranch) if('i386')
>
> That is all from me, but I think you need a buy-in from the Fortran
> maintainers (if they are ok with such an extension from Fortran language
> POV) and from Joseph (or other glibc people).
My main comment here is that if you go with the approach of a single
header shared by multilibs, you should also update the driver code so it
no longer uses any sort of multilib suffix when searching for this header
(it *should* still use the sysroot headers suffix when searching in a
sysroot).
--
Joseph S. Myers
jos...@codesourcery.com
Re: Fortran vector math header
On Thu, Jan 24, 2019 at 04:25:13PM +0100, Martin Liška wrote:
> @@ -11361,6 +11365,13 @@ gfc_match_gcc_builtin (void)
>else if (gfc_match (" ( inbranch ) ") == MATCH_YES)
> clause = SIMD_INBRANCH;
>
> + if (gfc_match (" if ( '%n' ) ", target) == MATCH_YES)
Not sure if it is clean enough to have the 's in there, rather than
matching there a string literal, requiring that it is a constant and getting
at the string in there. But if this is fine with the Fortran maintainers, I
won't object.
Given the patches for aarch64, I'd think that in addition to the target
specific multilib strings it might be worth to handle here also the generic
lp64, ilp32 and llp64 strings (by checking the precision of the
corresponding C types) and perhaps also big-endian, little-endian and
pdp-endian. This way, targets which have only those differences wouldn't
need to come up with their hooks.
> +! { dg-final { scan-tree-dump "sinf.simdclone" "optimized" { target ilp32 }
> } } */
> +! { dg-final { scan-tree-dump-not "sin.simdclone" "optimized" { target ilp32
> } } } */
I think you need ia32 here, otherwise it will fail on x32, which is also
ilp32 target, but not i386.
> +! { dg-final { scan-tree-dump "sin.simdclone" "optimized" { target lp64} } }
> */
> +! { dg-final { scan-tree-dump-not "sinf.simdclone" "optimized" { target lp64
> } } } */
> diff --git a/gcc/testsuite/gfortran.dg/simd-builtins-7.h
> b/gcc/testsuite/gfortran.dg/simd-builtins-7.h
> new file mode 100644
> index 000..1c19b88f877
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/simd-builtins-7.h
> @@ -0,0 +1,2 @@
> +!GCC$ builtin (sin) attributes simd (notinbranch) if('x86_64')
> +!GCC$ builtin (sinf) attributes simd (notinbranch) if('i386')
That is all from me, but I think you need a buy-in from the Fortran
maintainers (if they are ok with such an extension from Fortran language
POV) and from Joseph (or other glibc people).
Jakub
Re: Fortran vector math header
PING^1
On 1/24/19 4:25 PM, Martin Liška wrote:
> On 1/24/19 4:05 PM, Jakub Jelinek wrote:
>> On Thu, Jan 24, 2019 at 03:57:56PM +0100, Martin Liška wrote:
>>> --- a/gcc/config/i386/i386.c
>>> +++ b/gcc/config/i386/i386.c
>>> @@ -29577,6 +29577,17 @@ ix86_warn_parameter_passing_abi (cumulative_args_t
>>> cum_v, tree type)
>>>cum->warn_empty = false;
>>> }
>>>
>>> +static const char *
>>
>> Missing function comment. Usually a copy of the target hook description,
>> perhaps with arch details.
>>
>>> +ix86_get_multilib_abi_name (void)
>>> +{
>>> + if (!(TARGET_64BIT_P (ix86_isa_flags)))
>>> +return "i386";
>>> + else if (TARGET_X32_P (ix86_isa_flags))
>>> +return "x32";
>>> + else
>>> +return "x86_64";
>>> +}
>>> +
>>> /* Compute the alignment for a variable for Intel MCU psABI. TYPE is
>>> the data type, and ALIGN is the alignment that the object would
>>> ordinarily have. */
>>> @@ -51804,6 +51815,10 @@ ix86_run_selftests (void)
>>> #undef TARGET_WARN_PARAMETER_PASSING_ABI
>>> #define TARGET_WARN_PARAMETER_PASSING_ABI ix86_warn_parameter_passing_abi
>>>
>>> +#undef TARGET_GET_MULTILIB_ABI_NAME
>>> +#define TARGET_GET_MULTILIB_ABI_NAME \
>>> +ix86_get_multilib_abi_name
>>
>> All other #define TARGET_* that need to wrap line indent the next line
>> by two spaces, please do that too.
>>
>>> -/* Match a !GCC$ builtin (b) attributes simd flags form:
>>> +/* Match a !GCC$ builtin (b) attributes simd flags if(target) form:
>>>
>>> The parameter b is name of a middle-end built-in.
>>> - Flags are one of:
>>> - - (empty)
>>> + FLAGS is optional and must be one of:
>>> - inbranch
>>> - notinbranch
>>
>> FLAGS must be one of (inbranch) or (notinbranch) actually.
>>
>>> match
>>> gfc_match_gcc_builtin (void)
>>> {
>>>char builtin[GFC_MAX_SYMBOL_LEN + 1];
>>> + char target[GFC_MAX_SYMBOL_LEN + 1];
>>>
>>>if (gfc_match (" ( %n ) attributes simd", builtin) != MATCH_YES)
>>> return MATCH_ERROR;
>>> @@ -11361,6 +11365,13 @@ gfc_match_gcc_builtin (void)
>>>else if (gfc_match (" ( inbranch ) ") == MATCH_YES)
>>> clause = SIMD_INBRANCH;
>>>
>>> + if (gfc_match (" if ( %n ) ", target) == MATCH_YES)
>>> +{
>>> + const char *abi = targetm.get_multilib_abi_name ();
>>> + if (abi == NULL || strcmp (abi, target) != 0)
>>> + return MATCH_YES;
>>> +}
>>
>> Wonder whether we want if (x86_64) or if ('x86_64'), I'd lean for the
>> latter.
>>
>>> +
>>>if (gfc_vectorized_builtins == NULL)
>>> gfc_vectorized_builtins = new hash_map ();
>>>
>>> diff --git a/gcc/target.def b/gcc/target.def
>>> index 05c9cc1da28..4ba6b167e26 100644
>>> --- a/gcc/target.def
>>> +++ b/gcc/target.def
>>> @@ -5791,6 +5791,12 @@ call_2 may be NULL or a call insn.",
>>> rtx_insn *, (rtx_insn *call_1, rtx_insn *call_2),
>>> NULL)
>>>
>>> +DEFHOOK
>>> +(get_multilib_abi_name,
>>> + "This hook returns name of multilib ABI name.",
>>> + const char *, (void),
>>> + NULL)
>>> +
>>> DEFHOOK
>>> (remove_extra_call_preserved_regs,
>>> "This hook removes registers from the set of call-clobbered registers\n\
>>> diff --git a/gcc/targhooks.c b/gcc/targhooks.c
>>> index 529590b55df..a03b967b913 100644
>>> --- a/gcc/targhooks.c
>>> +++ b/gcc/targhooks.c
>>> @@ -2379,4 +2379,10 @@ default_remove_extra_call_preserved_regs (rtx_insn
>>> *, HARD_REG_SET *)
>>> {
>>> }
>>>
>>> +const char *
>>> +default_get_multilib_abi_name (void)
>>> +{
>>> + return NULL;
>>> +}
>>
>> Just use hook_constcharptr_void_null instead of adding yet another hook that
>> does the same thing?
>>
>> Jakub
>>
>
> Thanks for comments, I'm sending new version.
>
> Martin
>
Re: Fortran vector math header
On 1/24/19 10:24 PM, Steve Ellcey wrote:
> On Thu, 2019-01-24 at 16:25 +0100, Martin Liška wrote:
>>
>>>
+ix86_get_multilib_abi_name (void)
+{
+ if (!(TARGET_64BIT_P (ix86_isa_flags)))
+return "i386";
+ else if (TARGET_X32_P (ix86_isa_flags))
+return "x32";
+ else
+return "x86_64";
+}
>
>
> I'd like to get an aarch64 version of this target function into GCC 9.*
> too in order to support ILP32/LP64 on aarch64. It doesn't necessarily
> have to be part of this patch though, I could submit one later if you
> do not want to add it here.
Sure, feel free to provide a patch candidate. Once the patch is installed,
I'm planning to document format of the pre-include header file.
Martin
>
> Aarch64 ILP32 support is in GCC and binutils but not GLIBC (except on a
> branch), I am thinking the Aarch64 version of this function would
> return "aarch64" or "aarch64_ilp32". Perhaps we should also have
> "aarch64_be" and "aarch64_be_ilp32" for big endian ABI's?
>
> Steve Ellcey
> sell...@marvell.com
>
Re: Fortran vector math header
On Thu, 2019-01-24 at 16:25 +0100, Martin Liška wrote:
>
> >
> > > +ix86_get_multilib_abi_name (void)
> > > +{
> > > + if (!(TARGET_64BIT_P (ix86_isa_flags)))
> > > +return "i386";
> > > + else if (TARGET_X32_P (ix86_isa_flags))
> > > +return "x32";
> > > + else
> > > +return "x86_64";
> > > +}
I'd like to get an aarch64 version of this target function into GCC 9.*
too in order to support ILP32/LP64 on aarch64. It doesn't necessarily
have to be part of this patch though, I could submit one later if you
do not want to add it here.
Aarch64 ILP32 support is in GCC and binutils but not GLIBC (except on a
branch), I am thinking the Aarch64 version of this function would
return "aarch64" or "aarch64_ilp32". Perhaps we should also have
"aarch64_be" and "aarch64_be_ilp32" for big endian ABI's?
Steve Ellcey
sell...@marvell.com
Re: Fortran vector math header
On 1/24/19 4:05 PM, Jakub Jelinek wrote:
> On Thu, Jan 24, 2019 at 03:57:56PM +0100, Martin Liška wrote:
>> --- a/gcc/config/i386/i386.c
>> +++ b/gcc/config/i386/i386.c
>> @@ -29577,6 +29577,17 @@ ix86_warn_parameter_passing_abi (cumulative_args_t
>> cum_v, tree type)
>>cum->warn_empty = false;
>> }
>>
>> +static const char *
>
> Missing function comment. Usually a copy of the target hook description,
> perhaps with arch details.
>
>> +ix86_get_multilib_abi_name (void)
>> +{
>> + if (!(TARGET_64BIT_P (ix86_isa_flags)))
>> +return "i386";
>> + else if (TARGET_X32_P (ix86_isa_flags))
>> +return "x32";
>> + else
>> +return "x86_64";
>> +}
>> +
>> /* Compute the alignment for a variable for Intel MCU psABI. TYPE is
>> the data type, and ALIGN is the alignment that the object would
>> ordinarily have. */
>> @@ -51804,6 +51815,10 @@ ix86_run_selftests (void)
>> #undef TARGET_WARN_PARAMETER_PASSING_ABI
>> #define TARGET_WARN_PARAMETER_PASSING_ABI ix86_warn_parameter_passing_abi
>>
>> +#undef TARGET_GET_MULTILIB_ABI_NAME
>> +#define TARGET_GET_MULTILIB_ABI_NAME \
>> +ix86_get_multilib_abi_name
>
> All other #define TARGET_* that need to wrap line indent the next line
> by two spaces, please do that too.
>
>> -/* Match a !GCC$ builtin (b) attributes simd flags form:
>> +/* Match a !GCC$ builtin (b) attributes simd flags if(target) form:
>>
>> The parameter b is name of a middle-end built-in.
>> - Flags are one of:
>> - - (empty)
>> + FLAGS is optional and must be one of:
>> - inbranch
>> - notinbranch
>
> FLAGS must be one of (inbranch) or (notinbranch) actually.
>
>> match
>> gfc_match_gcc_builtin (void)
>> {
>>char builtin[GFC_MAX_SYMBOL_LEN + 1];
>> + char target[GFC_MAX_SYMBOL_LEN + 1];
>>
>>if (gfc_match (" ( %n ) attributes simd", builtin) != MATCH_YES)
>> return MATCH_ERROR;
>> @@ -11361,6 +11365,13 @@ gfc_match_gcc_builtin (void)
>>else if (gfc_match (" ( inbranch ) ") == MATCH_YES)
>> clause = SIMD_INBRANCH;
>>
>> + if (gfc_match (" if ( %n ) ", target) == MATCH_YES)
>> +{
>> + const char *abi = targetm.get_multilib_abi_name ();
>> + if (abi == NULL || strcmp (abi, target) != 0)
>> +return MATCH_YES;
>> +}
>
> Wonder whether we want if (x86_64) or if ('x86_64'), I'd lean for the
> latter.
>
>> +
>>if (gfc_vectorized_builtins == NULL)
>> gfc_vectorized_builtins = new hash_map ();
>>
>> diff --git a/gcc/target.def b/gcc/target.def
>> index 05c9cc1da28..4ba6b167e26 100644
>> --- a/gcc/target.def
>> +++ b/gcc/target.def
>> @@ -5791,6 +5791,12 @@ call_2 may be NULL or a call insn.",
>> rtx_insn *, (rtx_insn *call_1, rtx_insn *call_2),
>> NULL)
>>
>> +DEFHOOK
>> +(get_multilib_abi_name,
>> + "This hook returns name of multilib ABI name.",
>> + const char *, (void),
>> + NULL)
>> +
>> DEFHOOK
>> (remove_extra_call_preserved_regs,
>> "This hook removes registers from the set of call-clobbered registers\n\
>> diff --git a/gcc/targhooks.c b/gcc/targhooks.c
>> index 529590b55df..a03b967b913 100644
>> --- a/gcc/targhooks.c
>> +++ b/gcc/targhooks.c
>> @@ -2379,4 +2379,10 @@ default_remove_extra_call_preserved_regs (rtx_insn *,
>> HARD_REG_SET *)
>> {
>> }
>>
>> +const char *
>> +default_get_multilib_abi_name (void)
>> +{
>> + return NULL;
>> +}
>
> Just use hook_constcharptr_void_null instead of adding yet another hook that
> does the same thing?
>
> Jakub
>
Thanks for comments, I'm sending new version.
Martin
>From 3dce4e127c7810c4aff38f38ecb789fedc893082 Mon Sep 17 00:00:00 2001
From: marxin
Date: Mon, 21 Jan 2019 08:46:06 +0100
Subject: [PATCH] Support if statement in !GCC$ builtin directive.
gcc/ChangeLog:
2019-01-24 Martin Liska
* config/i386/i386.c (ix86_get_multilib_abi_name): New function.
(TARGET_GET_MULTILIB_ABI_NAME): New macro defined.
* doc/tm.texi: Document new target hook.
* doc/tm.texi.in: Likewise.
* target.def: Add new target macro.
gcc/fortran/ChangeLog:
2019-01-24 Martin Liska
* decl.c (gfc_match_gcc_builtin): Add support for filtering
of builtin directive based on multilib ABI name.
gcc/testsuite/ChangeLog:
2019-01-24 Martin Liska
* gfortran.dg/simd-builtins-7.f90: New test.
* gfortran.dg/simd-builtins-7.h: New test.
---
gcc/config/i386/i386.c| 17 +++
gcc/doc/tm.texi | 4
gcc/doc/tm.texi.in| 2 ++
gcc/fortran/decl.c| 21 ++-
gcc/target.def| 6 ++
gcc/testsuite/gfortran.dg/simd-builtins-7.f90 | 19 +
gcc/testsuite/gfortran.dg/simd-builtins-7.h | 2 ++
7 files changed, 66 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/simd-builtins-7.f90
create mode 100644 gcc/testsuite/gfortran.dg/simd-builtins-7.h
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index
Re: Fortran vector math header
On Thu, Jan 24, 2019 at 03:57:56PM +0100, Martin Liška wrote:
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -29577,6 +29577,17 @@ ix86_warn_parameter_passing_abi (cumulative_args_t
> cum_v, tree type)
>cum->warn_empty = false;
> }
>
> +static const char *
Missing function comment. Usually a copy of the target hook description,
perhaps with arch details.
> +ix86_get_multilib_abi_name (void)
> +{
> + if (!(TARGET_64BIT_P (ix86_isa_flags)))
> +return "i386";
> + else if (TARGET_X32_P (ix86_isa_flags))
> +return "x32";
> + else
> +return "x86_64";
> +}
> +
> /* Compute the alignment for a variable for Intel MCU psABI. TYPE is
> the data type, and ALIGN is the alignment that the object would
> ordinarily have. */
> @@ -51804,6 +51815,10 @@ ix86_run_selftests (void)
> #undef TARGET_WARN_PARAMETER_PASSING_ABI
> #define TARGET_WARN_PARAMETER_PASSING_ABI ix86_warn_parameter_passing_abi
>
> +#undef TARGET_GET_MULTILIB_ABI_NAME
> +#define TARGET_GET_MULTILIB_ABI_NAME \
> +ix86_get_multilib_abi_name
All other #define TARGET_* that need to wrap line indent the next line
by two spaces, please do that too.
> -/* Match a !GCC$ builtin (b) attributes simd flags form:
> +/* Match a !GCC$ builtin (b) attributes simd flags if(target) form:
>
> The parameter b is name of a middle-end built-in.
> - Flags are one of:
> - - (empty)
> + FLAGS is optional and must be one of:
> - inbranch
> - notinbranch
FLAGS must be one of (inbranch) or (notinbranch) actually.
> match
> gfc_match_gcc_builtin (void)
> {
>char builtin[GFC_MAX_SYMBOL_LEN + 1];
> + char target[GFC_MAX_SYMBOL_LEN + 1];
>
>if (gfc_match (" ( %n ) attributes simd", builtin) != MATCH_YES)
> return MATCH_ERROR;
> @@ -11361,6 +11365,13 @@ gfc_match_gcc_builtin (void)
>else if (gfc_match (" ( inbranch ) ") == MATCH_YES)
> clause = SIMD_INBRANCH;
>
> + if (gfc_match (" if ( %n ) ", target) == MATCH_YES)
> +{
> + const char *abi = targetm.get_multilib_abi_name ();
> + if (abi == NULL || strcmp (abi, target) != 0)
> + return MATCH_YES;
> +}
Wonder whether we want if (x86_64) or if ('x86_64'), I'd lean for the
latter.
> +
>if (gfc_vectorized_builtins == NULL)
> gfc_vectorized_builtins = new hash_map ();
>
> diff --git a/gcc/target.def b/gcc/target.def
> index 05c9cc1da28..4ba6b167e26 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -5791,6 +5791,12 @@ call_2 may be NULL or a call insn.",
> rtx_insn *, (rtx_insn *call_1, rtx_insn *call_2),
> NULL)
>
> +DEFHOOK
> +(get_multilib_abi_name,
> + "This hook returns name of multilib ABI name.",
> + const char *, (void),
> + NULL)
> +
> DEFHOOK
> (remove_extra_call_preserved_regs,
> "This hook removes registers from the set of call-clobbered registers\n\
> diff --git a/gcc/targhooks.c b/gcc/targhooks.c
> index 529590b55df..a03b967b913 100644
> --- a/gcc/targhooks.c
> +++ b/gcc/targhooks.c
> @@ -2379,4 +2379,10 @@ default_remove_extra_call_preserved_regs (rtx_insn *,
> HARD_REG_SET *)
> {
> }
>
> +const char *
> +default_get_multilib_abi_name (void)
> +{
> + return NULL;
> +}
Just use hook_constcharptr_void_null instead of adding yet another hook that
does the same thing?
Jakub
Re: Fortran vector math header
On 1/23/19 2:07 AM, Joseph Myers wrote:
> On Tue, 22 Jan 2019, Richard Biener wrote:
>
>>> Or instead just come up with target specific strings to determine the ABI,
>>> say i386, x86_64 and x32 for the 3 ABIs on x86, powerpc{,64}{,le} on rs6000
>>> etc.
>>
>> Yeah, I would even suggest to use a target hook multilib_ABI_active_p
>> (const char *)
>> for this ... (where the hook should diagnose unknown multilib specifiers).
>
> A header from a newer glibc version, supporting new ABIs, should still be
> usable with an older GCC version, not supporting some of those ABIs.
> Thus I don't think the hook should diagnose unknown specifiers from such a
> header. (This is just like how a C header can test new predefined macros
> without an old GCC complaining it doesn't know what would define those
> macros.)
>
Hi.
I'm sending patch candidate that introduced a new target hook.
Thoughts?
Martin
>From 46dd8bad4ce4c467e7387b75ba8241a3de194b71 Mon Sep 17 00:00:00 2001
From: marxin
Date: Mon, 21 Jan 2019 08:46:06 +0100
Subject: [PATCH] Support if statement in !GCC$ builtin directive.
gcc/ChangeLog:
2019-01-24 Martin Liska
* config/i386/i386.c (ix86_get_multilib_abi_name): New function.
(TARGET_GET_MULTILIB_ABI_NAME): New macro defined.
* doc/tm.texi: Document new target hook.
* doc/tm.texi.in: Likewise.
* target.def: Add new target macro.
* targhooks.c (default_get_multilib_abi_name): New.
* targhooks.h (default_get_multilib_abi_name): Likewise.
gcc/fortran/ChangeLog:
2019-01-24 Martin Liska
* decl.c (gfc_match_gcc_builtin): Add support for filtering
of builtin directive based on multilib ABI name.
gcc/testsuite/ChangeLog:
2019-01-24 Martin Liska
* gfortran.dg/simd-builtins-7.f90: New test.
* gfortran.dg/simd-builtins-7.h: New test.
---
gcc/config/i386/i386.c| 15 +++
gcc/doc/tm.texi | 4
gcc/doc/tm.texi.in| 2 ++
gcc/fortran/decl.c| 17 ++---
gcc/target.def| 6 ++
gcc/targhooks.c | 6 ++
gcc/targhooks.h | 1 +
gcc/testsuite/gfortran.dg/simd-builtins-7.f90 | 19 +++
gcc/testsuite/gfortran.dg/simd-builtins-7.h | 2 ++
9 files changed, 69 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/simd-builtins-7.f90
create mode 100644 gcc/testsuite/gfortran.dg/simd-builtins-7.h
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index e0d7c74fcec..95fc905b4ea 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -29577,6 +29577,17 @@ ix86_warn_parameter_passing_abi (cumulative_args_t cum_v, tree type)
cum->warn_empty = false;
}
+static const char *
+ix86_get_multilib_abi_name (void)
+{
+ if (!(TARGET_64BIT_P (ix86_isa_flags)))
+return "i386";
+ else if (TARGET_X32_P (ix86_isa_flags))
+return "x32";
+ else
+return "x86_64";
+}
+
/* Compute the alignment for a variable for Intel MCU psABI. TYPE is
the data type, and ALIGN is the alignment that the object would
ordinarily have. */
@@ -51804,6 +51815,10 @@ ix86_run_selftests (void)
#undef TARGET_WARN_PARAMETER_PASSING_ABI
#define TARGET_WARN_PARAMETER_PASSING_ABI ix86_warn_parameter_passing_abi
+#undef TARGET_GET_MULTILIB_ABI_NAME
+#define TARGET_GET_MULTILIB_ABI_NAME \
+ix86_get_multilib_abi_name
+
#if CHECKING_P
#undef TARGET_RUN_TARGET_SELFTESTS
#define TARGET_RUN_TARGET_SELFTESTS selftest::ix86_run_selftests
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 4347f89cd2f..8c8978bb13a 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -1931,6 +1931,10 @@ superset of all other ABIs. @var{call_1} must always be a call insn,
call_2 may be NULL or a call insn.
@end deftypefn
+@deftypefn {Target Hook} {const char *} TARGET_GET_MULTILIB_ABI_NAME (void)
+This hook returns name of multilib ABI name.
+@end deftypefn
+
@findex fixed_regs
@findex call_used_regs
@findex global_regs
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 41a6cb11cb0..fe1194ef91a 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -1711,6 +1711,8 @@ of @code{CALL_USED_REGISTERS}.
@hook TARGET_RETURN_CALL_WITH_MAX_CLOBBERS
+@hook TARGET_GET_MULTILIB_ABI_NAME
+
@findex fixed_regs
@findex call_used_regs
@findex global_regs
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 3314e176881..81e4bfcf094 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -28,6 +28,7 @@ along with GCC; see the file COPYING3. If not see
#include "match.h"
#include "parse.h"
#include "constructor.h"
+#include "target.h"
/* Macros to access allocate memory for gfc_data_variable,
gfc_data_value and gfc_data. */
@@ -11338,19 +11339,22 @@ gfc_match_gcc_unroll (void)
return MATCH_ERROR;
}
-/* Match a !GCC$ builtin (b) attributes simd flags form:
+/* Match a !GCC$ builtin (b) attributes simd
Re: [EXT] Re: Fortran vector math header
On Wed, 2019-01-23 at 23:53 +0100, Jakub Jelinek wrote:
> External Email
>
> ---
> ---
> On Wed, Jan 23, 2019 at 09:56:21PM +, Steve Ellcey wrote:
> > I wonder if we even need to pass a string to the target
> > hook. Instead
> > of:
> >
> > !GCC$ builtin (cos) attributes simd (notinbranch) if('x86_64-linux-gnu')
> >
> > We just have:
> >
> > !GCC$ builtin (cos) attributes simd (notinbranch) if_allowed
>
> That is not possible. The whole point why we have a header is that glibc
> as the library provider provides a header which says to gcc which builtins
> have simd entrypoints, for which target and for which multilib.
>
> If GCC knew or wanted to hardcode that info, we wouldn't have a header, we'd
> just hardcode it.
> The point is that at some later point, glibc might get an implementation for
> say powerpc64 64-bit, or mips n32, or whatever else, say for a subset of the
> builtins x86_64 has, or superset, ...
Duh. Of course. I am not sure how I lost track of that fact.
Steve Ellcey
sell...@marvell.com
Re: Fortran vector math header
On Wed, Jan 23, 2019 at 09:56:21PM +, Steve Ellcey wrote:
> I wonder if we even need to pass a string to the target hook. Instead
> of:
>
> !GCC$ builtin (cos) attributes simd (notinbranch) if('x86_64-linux-gnu')
>
> We just have:
>
> !GCC$ builtin (cos) attributes simd (notinbranch) if_allowed
That is not possible. The whole point why we have a header is that glibc
as the library provider provides a header which says to gcc which builtins
have simd entrypoints, for which target and for which multilib.
If GCC knew or wanted to hardcode that info, we wouldn't have a header, we'd
just hardcode it.
The point is that at some later point, glibc might get an implementation for
say powerpc64 64-bit, or mips n32, or whatever else, say for a subset of the
builtins x86_64 has, or superset, ...
Jakub
Re: Fortran vector math header
On Tue, 2019-01-22 at 13:18 +0100, Richard Biener wrote:
>
> Yeah, I would even suggest to use a target hook multilib_ABI_active_p
> (const char *)
> for this ... (where the hook should diagnose unknown multilib specifiers).
>
> Richard.
I wonder if we even need to pass a string to the target hook. Instead
of:
!GCC$ builtin (cos) attributes simd (notinbranch) if('x86_64-linux-gnu')
We just have:
!GCC$ builtin (cos) attributes simd (notinbranch) if_allowed
When we see if_allowed we call a target function like multilib_ABI_active_p
and it can use whatever criteria it wants to use to determine if the simd
attribute should be honored.
Or, if we are going this far, how about leaving out the if altogher and
everytime we see a simd attribute in Fortran we call a target function
to determine if it should or should not be honored. The default function
can just return true, target specific versions could look at the ABI and
return true or false as needed.
It might also be worth passing the function (cos) as an argument to the
target function, then we could have different ABI's enabling different
functions and not have to have them all on or all off based on the ABI.
Steve Ellcey
sell...@marvell.com
Re: Fortran vector math header
On Tue, 22 Jan 2019, Richard Biener wrote:
> > Or instead just come up with target specific strings to determine the ABI,
> > say i386, x86_64 and x32 for the 3 ABIs on x86, powerpc{,64}{,le} on rs6000
> > etc.
>
> Yeah, I would even suggest to use a target hook multilib_ABI_active_p
> (const char *)
> for this ... (where the hook should diagnose unknown multilib specifiers).
A header from a newer glibc version, supporting new ABIs, should still be
usable with an older GCC version, not supporting some of those ABIs.
Thus I don't think the hook should diagnose unknown specifiers from such a
header. (This is just like how a C header can test new predefined macros
without an old GCC complaining it doesn't know what would define those
macros.)
--
Joseph S. Myers
jos...@codesourcery.com
Re: Fortran vector math header
On Tue, Jan 22, 2019 at 12:15 PM Jakub Jelinek wrote:
>
> On Tue, Jan 22, 2019 at 12:01:29PM +0100, Martin Liška wrote:
> > I discussed with Jakub possibility of generating canonical triples that will
> > be then used in Fortran FE to filter the 'GCC$ builtin' directives. However,
> > the approach bring some possibilities and thus I enhanced the previous
> > version of patch to catch x32 ABI (I call it lp64ilp32), similarly to
> > Debian:
>
> lp64ilp32 makes no sense. long and pointer are 64-bit and integer, long and
> pointer are 32-bit? That is impossible. Both ia32 and x32 are ilp32.
>
> What I was suggesting is either use normalized tuples like Debian
> multi-arch, but as it would be exposed to users on all targets, it would
> need to be something done for all targets. My suggestion was start with
> the canonical triplet and adjust it for multilibs in a target hook if
> needed. So, e.g. in config/i386, it would replace the first part of the
> triplet with x86_64 if not ia32, if ia32 with i386, and append x32 to the
> triplet if x32. For e.g. rs6000, it could modify the first part of the
> triplet to powerpc{,64}{,le} depending on endianity and if it is 64-bit ABI
> or not. For aarch64 it would modify the first part of the triplet to
> aarch64{,be} depending on endianity, and append _ilp32 if 32-bit ABI, etc.
>
> Or instead just come up with target specific strings to determine the ABI,
> say i386, x86_64 and x32 for the 3 ABIs on x86, powerpc{,64}{,le} on rs6000
> etc.
Yeah, I would even suggest to use a target hook multilib_ABI_active_p
(const char *)
for this ... (where the hook should diagnose unknown multilib specifiers).
Richard.
> Jakub
Re: Fortran vector math header
On Tue, Jan 22, 2019 at 12:01:29PM +0100, Martin Liška wrote:
> I discussed with Jakub possibility of generating canonical triples that will
> be then used in Fortran FE to filter the 'GCC$ builtin' directives. However,
> the approach bring some possibilities and thus I enhanced the previous
> version of patch to catch x32 ABI (I call it lp64ilp32), similarly to Debian:
lp64ilp32 makes no sense. long and pointer are 64-bit and integer, long and
pointer are 32-bit? That is impossible. Both ia32 and x32 are ilp32.
What I was suggesting is either use normalized tuples like Debian
multi-arch, but as it would be exposed to users on all targets, it would
need to be something done for all targets. My suggestion was start with
the canonical triplet and adjust it for multilibs in a target hook if
needed. So, e.g. in config/i386, it would replace the first part of the
triplet with x86_64 if not ia32, if ia32 with i386, and append x32 to the
triplet if x32. For e.g. rs6000, it could modify the first part of the
triplet to powerpc{,64}{,le} depending on endianity and if it is 64-bit ABI
or not. For aarch64 it would modify the first part of the triplet to
aarch64{,be} depending on endianity, and append _ilp32 if 32-bit ABI, etc.
Or instead just come up with target specific strings to determine the ABI,
say i386, x86_64 and x32 for the 3 ABIs on x86, powerpc{,64}{,le} on rs6000
etc.
Jakub
Re: Fortran vector math header
On 1/21/19 3:25 PM, Joseph Myers wrote:
> On Mon, 21 Jan 2019, Martin Liška wrote:
>
>> I like the if('lp64'), if('ilp32') approach and I'm sending patch
>> candidate for that. Would it be accepted by glibc folks?
>
> Since glibc supports libmvec for x86_64, both 64-bit and x32, but not for
> 32-bit x86, those particular conditionals are insufficient because they
> can't distinguish x32 (libmvec supported) from 32-bit x86 (libmvec not
> supported); you need some architecture-specific conditional that can be
> used in the Fortran header.
>
Hello.
I discussed with Jakub possibility of generating canonical triples that will
be then used in Fortran FE to filter the 'GCC$ builtin' directives. However,
the approach bring some possibilities and thus I enhanced the previous
version of patch to catch x32 ABI (I call it lp64ilp32), similarly to Debian:
https://wiki.debian.org/Multiarch/Tuples
If I'm correct, such ABI can be provided on i386, aarch64 and mips64 targets.
Will the suggested approach work?
Martin
>From d585da8b477ac13be1941d290d7e2b8c7b5bf96a Mon Sep 17 00:00:00 2001
From: marxin
Date: Mon, 21 Jan 2019 08:46:06 +0100
Subject: [PATCH] Support if statement in !GCC$ builtin directive.
gcc/fortran/ChangeLog:
2019-01-21 Martin Liska
* decl.c (gfc_match_gcc_builtin): Support filtering for
'lp64' and 'ilp32'.
gcc/testsuite/ChangeLog:
2019-01-21 Martin Liska
* gfortran.dg/simd-builtins-7.f90: New test.
* gfortran.dg/simd-builtins-7.h: New test.
---
gcc/config/i386/i386.c| 15
gcc/coretypes.h | 9 +
gcc/doc/tm.texi | 4 +++
gcc/doc/tm.texi.in| 2 ++
gcc/fortran/decl.c| 35 +--
gcc/target.def| 6
gcc/targhooks.c | 6
gcc/targhooks.h | 1 +
gcc/testsuite/gfortran.dg/simd-builtins-7.f90 | 19 ++
gcc/testsuite/gfortran.dg/simd-builtins-7.h | 2 ++
10 files changed, 96 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/simd-builtins-7.f90
create mode 100644 gcc/testsuite/gfortran.dg/simd-builtins-7.h
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 8abff99cc62..0167b4bd3e5 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -29573,6 +29573,17 @@ ix86_warn_parameter_passing_abi (cumulative_args_t cum_v, tree type)
cum->warn_empty = false;
}
+static target_abi_type
+ix86_get_abi_type (void)
+{
+ if (!(TARGET_64BIT_P (ix86_isa_flags )))
+return ABI_ILP32;
+ else if (TARGET_X32_P (ix86_isa_flags))
+return ABI_LP64ILP32;
+ else
+return ABI_LP64;
+}
+
/* Compute the alignment for a variable for Intel MCU psABI. TYPE is
the data type, and ALIGN is the alignment that the object would
ordinarily have. */
@@ -51900,6 +51911,10 @@ ix86_run_selftests (void)
#undef TARGET_WARN_PARAMETER_PASSING_ABI
#define TARGET_WARN_PARAMETER_PASSING_ABI ix86_warn_parameter_passing_abi
+#undef TARGET_GET_ABI_TYPE
+#define TARGET_GET_ABI_TYPE \
+ix86_get_abi_type
+
#if CHECKING_P
#undef TARGET_RUN_TARGET_SELFTESTS
#define TARGET_RUN_TARGET_SELFTESTS selftest::ix86_run_selftests
diff --git a/gcc/coretypes.h b/gcc/coretypes.h
index 2f6b8599d7c..ba3fe9c2e7c 100644
--- a/gcc/coretypes.h
+++ b/gcc/coretypes.h
@@ -290,6 +290,15 @@ enum warn_strict_overflow_code
WARN_STRICT_OVERFLOW_MAGNITUDE = 5
};
+/* Enum values for target ABI types. */
+enum target_abi_type
+{
+ ABI_LP64,
+ ABI_ILP32,
+ ABI_LP64ILP32,
+ ABI_OTHER
+};
+
/* The type of an alias set. Code currently assumes that variables of
this type can take the values 0 (the alias set which aliases
everything) and -1 (sometimes indicating that the alias set is
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 4347f89cd2f..7887217f10a 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -1931,6 +1931,10 @@ superset of all other ABIs. @var{call_1} must always be a call insn,
call_2 may be NULL or a call insn.
@end deftypefn
+@deftypefn {Target Hook} {enum target_abi_type} TARGET_GET_ABI_TYPE (void)
+This hooks returns type of target ABI (LP64, ILP32, LP64ILP32 or other).
+@end deftypefn
+
@findex fixed_regs
@findex call_used_regs
@findex global_regs
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 41a6cb11cb0..b36a2064290 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -1711,6 +1711,8 @@ of @code{CALL_USED_REGISTERS}.
@hook TARGET_RETURN_CALL_WITH_MAX_CLOBBERS
+@hook TARGET_GET_ABI_TYPE
+
@findex fixed_regs
@findex call_used_regs
@findex global_regs
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 3314e176881..f4efc3140e3 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -28,6 +28,8 @@ along with GCC; see the file COPYING3. If not see
#include "match.h"
#include "parse.h"
#include "constructor.h"
Re: Fortran vector math header
On Mon, 21 Jan 2019, Martin Liška wrote:
> I like the if('lp64'), if('ilp32') approach and I'm sending patch
> candidate for that. Would it be accepted by glibc folks?
Since glibc supports libmvec for x86_64, both 64-bit and x32, but not for
32-bit x86, those particular conditionals are insufficient because they
can't distinguish x32 (libmvec supported) from 32-bit x86 (libmvec not
supported); you need some architecture-specific conditional that can be
used in the Fortran header.
--
Joseph S. Myers
jos...@codesourcery.com
Re: Fortran vector math header
On Mon, Jan 21, 2019 at 10:36:20AM +0100, Martin Liška wrote: > @@ -11338,7 +11339,7 @@ gfc_match_gcc_unroll (void) >return MATCH_ERROR; > } > > -/* Match a !GCC$ builtin (b) attributes simd flags form: > +/* Match a !GCC$ builtin (b) attributes simd if(target) flags form: > > The parameter b is name of a middle-end built-in. > Flags are one of: > @@ -11346,11 +11347,16 @@ gfc_match_gcc_unroll (void) > - inbranch > - notinbranch > > + Target must be one of: > + - lp64 > + - ilp32 > + Not exactly correct. if(target) is optional, not mandatory, and, if it appears, must appear after flags. flags is also incorrect, because it is either nothing, or (inbranch), or (notinbranch), while the function comment suggests that it is inbranch or notinbranch. Jakub
Re: Fortran vector math header
On 1/21/19 10:19 AM, Jakub Jelinek wrote:
> On Mon, Jan 21, 2019 at 10:09:01AM +0100, Martin Liška wrote:
>> @@ -11351,6 +11352,7 @@ match
>> gfc_match_gcc_builtin (void)
>> {
>>char builtin[GFC_MAX_SYMBOL_LEN + 1];
>> + char target[GFC_MAX_SYMBOL_LEN + 1];
>>
>>if (gfc_match (" ( %n ) attributes simd", builtin) != MATCH_YES)
>> return MATCH_ERROR;
>> @@ -11361,6 +11363,26 @@ gfc_match_gcc_builtin (void)
>>else if (gfc_match (" ( inbranch ) ") == MATCH_YES)
>> clause = SIMD_INBRANCH;
>>
>> + if (gfc_match (" if ( %n ) ", target) == MATCH_YES)
>> +{
>> + if (strcmp (target, "lp64") == 0)
>> +{
>> + if (TYPE_PRECISION (long_integer_type_node) != 64
>> + || POINTER_SIZE != 64
>> + || TYPE_PRECISION (integer_type_node) != 32)
>> +return MATCH_YES;
>> +}
>> + else if (strcmp (target, "ilp32") == 0)
>> +{
>> + if (TYPE_PRECISION (long_integer_type_node) != 32
>> + || POINTER_SIZE != 32
>> + || TYPE_PRECISION (integer_type_node) != 32)
>> +return MATCH_YES;
>> +}
>> + else
>> +return MATCH_ERROR;
>> +}
>
> You should adjust the syntax above the function too.
> And you need here buy-in from glibc folks.
>> +
>>if (gfc_vectorized_builtins == NULL)
>> gfc_vectorized_builtins = new hash_map ();
>>
>> diff --git a/gcc/testsuite/gfortran.dg/simd-builtins-7.f90
>> b/gcc/testsuite/gfortran.dg/simd-builtins-7.f90
>> new file mode 100644
>> index 000..6445733d288
>> --- /dev/null
>> +++ b/gcc/testsuite/gfortran.dg/simd-builtins-7.f90
>> @@ -0,0 +1,21 @@
>> +! { dg-do compile { target { i?86-*-linux* x86_64-*-linux* } } }
>> +! { dg-additional-options "-msse2 -mno-avx -nostdinc -Ofast
>> -fpre-include=simd-builtins-7.h -fdump-tree-optimized" }
>> +
>> +program test_overloaded_intrinsic
>> + real(4) :: x4(3200), y4(3200)
>> + real(8) :: x8(3200), y8(3200)
>> +
>> + ! this should be using simd clone
>> + y4 = sin(x4)
>> + print *, y4
>> +
>> + ! this should not be using simd clone
>
> The above 2 comments are misleading, they only match what ia32 does.
>
>> + y4 = sin(x8)
>> + print *, y8
>> +end
>> +
>> +! { dg-final { scan-tree-dump "sinf.simdclone" "optimized" { target ia32 }
>> } } */
>> +! { dg-final { scan-tree-dump-not "sin.simdclone" "optimized" { target ia32
>> } } } */
>
> Perhaps use ilp32 instead of ia32?
>
> Jakub
>
Thanks for review, I'm attaching updated patch.
Martin
>From f8901f176f7832591dd9702da681859b9f3ddf38 Mon Sep 17 00:00:00 2001
From: marxin
Date: Mon, 21 Jan 2019 08:46:06 +0100
Subject: [PATCH] Support if statement in !GCC$ builtin directive.
gcc/fortran/ChangeLog:
2019-01-21 Martin Liska
* decl.c (gfc_match_gcc_builtin): Support filtering for
'lp64' and 'ilp32'.
gcc/testsuite/ChangeLog:
2019-01-21 Martin Liska
* gfortran.dg/simd-builtins-7.f90: New test.
* gfortran.dg/simd-builtins-7.h: New test.
---
gcc/fortran/decl.c| 28 ++-
gcc/testsuite/gfortran.dg/simd-builtins-7.f90 | 19 +
gcc/testsuite/gfortran.dg/simd-builtins-7.h | 2 ++
3 files changed, 48 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gfortran.dg/simd-builtins-7.f90
create mode 100644 gcc/testsuite/gfortran.dg/simd-builtins-7.h
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 3314e176881..3e2e9adcb4a 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -28,6 +28,7 @@ along with GCC; see the file COPYING3. If not see
#include "match.h"
#include "parse.h"
#include "constructor.h"
+#include "target.h"
/* Macros to access allocate memory for gfc_data_variable,
gfc_data_value and gfc_data. */
@@ -11338,7 +11339,7 @@ gfc_match_gcc_unroll (void)
return MATCH_ERROR;
}
-/* Match a !GCC$ builtin (b) attributes simd flags form:
+/* Match a !GCC$ builtin (b) attributes simd if(target) flags form:
The parameter b is name of a middle-end built-in.
Flags are one of:
@@ -11346,11 +11347,16 @@ gfc_match_gcc_unroll (void)
- inbranch
- notinbranch
+ Target must be one of:
+ - lp64
+ - ilp32
+
When we come here, we have already matched the !GCC$ builtin string. */
match
gfc_match_gcc_builtin (void)
{
char builtin[GFC_MAX_SYMBOL_LEN + 1];
+ char target[GFC_MAX_SYMBOL_LEN + 1];
if (gfc_match (" ( %n ) attributes simd", builtin) != MATCH_YES)
return MATCH_ERROR;
@@ -11361,6 +11367,26 @@ gfc_match_gcc_builtin (void)
else if (gfc_match (" ( inbranch ) ") == MATCH_YES)
clause = SIMD_INBRANCH;
+ if (gfc_match (" if ( %n ) ", target) == MATCH_YES)
+{
+ if (strcmp (target, "lp64") == 0)
+ {
+ if (TYPE_PRECISION (long_integer_type_node) != 64
+ || POINTER_SIZE != 64
+ || TYPE_PRECISION (integer_type_node) != 32)
+ return MATCH_YES;
+ }
+ else if (strcmp (target, "ilp32") == 0)
+ {
+ if (TYPE_PRECISION (long_integer_type_node) != 32
+ || POINTER_SIZE != 32
Re: Fortran vector math header
On Mon, Jan 21, 2019 at 10:09:01AM +0100, Martin Liška wrote:
> @@ -11351,6 +11352,7 @@ match
> gfc_match_gcc_builtin (void)
> {
>char builtin[GFC_MAX_SYMBOL_LEN + 1];
> + char target[GFC_MAX_SYMBOL_LEN + 1];
>
>if (gfc_match (" ( %n ) attributes simd", builtin) != MATCH_YES)
> return MATCH_ERROR;
> @@ -11361,6 +11363,26 @@ gfc_match_gcc_builtin (void)
>else if (gfc_match (" ( inbranch ) ") == MATCH_YES)
> clause = SIMD_INBRANCH;
>
> + if (gfc_match (" if ( %n ) ", target) == MATCH_YES)
> +{
> + if (strcmp (target, "lp64") == 0)
> + {
> + if (TYPE_PRECISION (long_integer_type_node) != 64
> + || POINTER_SIZE != 64
> + || TYPE_PRECISION (integer_type_node) != 32)
> + return MATCH_YES;
> + }
> + else if (strcmp (target, "ilp32") == 0)
> + {
> + if (TYPE_PRECISION (long_integer_type_node) != 32
> + || POINTER_SIZE != 32
> + || TYPE_PRECISION (integer_type_node) != 32)
> + return MATCH_YES;
> + }
> + else
> + return MATCH_ERROR;
> +}
You should adjust the syntax above the function too.
And you need here buy-in from glibc folks.
> +
>if (gfc_vectorized_builtins == NULL)
> gfc_vectorized_builtins = new hash_map ();
>
> diff --git a/gcc/testsuite/gfortran.dg/simd-builtins-7.f90
> b/gcc/testsuite/gfortran.dg/simd-builtins-7.f90
> new file mode 100644
> index 000..6445733d288
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/simd-builtins-7.f90
> @@ -0,0 +1,21 @@
> +! { dg-do compile { target { i?86-*-linux* x86_64-*-linux* } } }
> +! { dg-additional-options "-msse2 -mno-avx -nostdinc -Ofast
> -fpre-include=simd-builtins-7.h -fdump-tree-optimized" }
> +
> +program test_overloaded_intrinsic
> + real(4) :: x4(3200), y4(3200)
> + real(8) :: x8(3200), y8(3200)
> +
> + ! this should be using simd clone
> + y4 = sin(x4)
> + print *, y4
> +
> + ! this should not be using simd clone
The above 2 comments are misleading, they only match what ia32 does.
> + y4 = sin(x8)
> + print *, y8
> +end
> +
> +! { dg-final { scan-tree-dump "sinf.simdclone" "optimized" { target ia32 } }
> } */
> +! { dg-final { scan-tree-dump-not "sin.simdclone" "optimized" { target ia32
> } } } */
Perhaps use ilp32 instead of ia32?
Jakub
Re: Fortran vector math header
On 1/21/19 8:58 AM, Jakub Jelinek wrote:
> On Mon, Jan 21, 2019 at 08:47:42AM +0100, Martin Liška wrote:
>> diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
>> index 3314e176881..5c7c062574b 100644
>> --- a/gcc/fortran/decl.c
>> +++ b/gcc/fortran/decl.c
>> @@ -11351,6 +11351,7 @@ match
>> gfc_match_gcc_builtin (void)
>> {
>>char builtin[GFC_MAX_SYMBOL_LEN + 1];
>> + char target[GFC_MAX_SYMBOL_LEN + 1];
>>
>>if (gfc_match (" ( %n ) attributes simd", builtin) != MATCH_YES)
>> return MATCH_ERROR;
>> @@ -11361,6 +11362,24 @@ gfc_match_gcc_builtin (void)
>>else if (gfc_match (" ( inbranch ) ") == MATCH_YES)
>> clause = SIMD_INBRANCH;
>>
>> + if (gfc_match (" if ( %n ) ", target) == MATCH_YES)
>> +{
>> + unsigned HOST_WIDE_INT size_of_long
>> += tree_to_uhwi (TYPE_SIZE_UNIT (long_integer_type_node));
>> + if (strcmp (target, "lp64") == 0)
>> +{
>> + if (size_of_long != 8)
>> +return MATCH_YES;
>
> __LP64__ macro is defined if:
> if (TYPE_PRECISION (long_integer_type_node) == 64
> && POINTER_SIZE == 64
> && TYPE_PRECISION (integer_type_node) == 32)
> All of these are middle-end types or target macros, so you should be able to
> test that in the Fortran FE too.
>
>> +}
>> + else if (strcmp (target, "ilp32") == 0)
>> +{
>> + if (size_of_long != 4)
>> +return MATCH_YES;
>
> For this obviously as well.
>
>> +}
>> + else
>> +return MATCH_ERROR;
>> +}
>> +
>>if (gfc_vectorized_builtins == NULL)
>> gfc_vectorized_builtins = new hash_map ();
>
> Jakub
>
Sure, this is patch I've been currently testing.
Thoughts?
Martin
>From 1520b49856a1b39358602c4105dda3891cb166ff Mon Sep 17 00:00:00 2001
From: marxin
Date: Mon, 21 Jan 2019 08:46:06 +0100
Subject: [PATCH] Support if statement in !GCC$ builtin directive.
gcc/fortran/ChangeLog:
2019-01-21 Martin Liska
* decl.c (gfc_match_gcc_builtin): Support filtering for
'lp64' and 'ilp32'.
gcc/testsuite/ChangeLog:
2019-01-21 Martin Liska
* gfortran.dg/simd-builtins-7.f90: New test.
* gfortran.dg/simd-builtins-7.h: New test.
---
gcc/fortran/decl.c| 22 +++
gcc/testsuite/gfortran.dg/simd-builtins-7.f90 | 21 ++
gcc/testsuite/gfortran.dg/simd-builtins-7.h | 2 ++
3 files changed, 45 insertions(+)
create mode 100644 gcc/testsuite/gfortran.dg/simd-builtins-7.f90
create mode 100644 gcc/testsuite/gfortran.dg/simd-builtins-7.h
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 3314e176881..d68ab8e7931 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -28,6 +28,7 @@ along with GCC; see the file COPYING3. If not see
#include "match.h"
#include "parse.h"
#include "constructor.h"
+#include "target.h"
/* Macros to access allocate memory for gfc_data_variable,
gfc_data_value and gfc_data. */
@@ -11351,6 +11352,7 @@ match
gfc_match_gcc_builtin (void)
{
char builtin[GFC_MAX_SYMBOL_LEN + 1];
+ char target[GFC_MAX_SYMBOL_LEN + 1];
if (gfc_match (" ( %n ) attributes simd", builtin) != MATCH_YES)
return MATCH_ERROR;
@@ -11361,6 +11363,26 @@ gfc_match_gcc_builtin (void)
else if (gfc_match (" ( inbranch ) ") == MATCH_YES)
clause = SIMD_INBRANCH;
+ if (gfc_match (" if ( %n ) ", target) == MATCH_YES)
+{
+ if (strcmp (target, "lp64") == 0)
+ {
+ if (TYPE_PRECISION (long_integer_type_node) != 64
+ || POINTER_SIZE != 64
+ || TYPE_PRECISION (integer_type_node) != 32)
+ return MATCH_YES;
+ }
+ else if (strcmp (target, "ilp32") == 0)
+ {
+ if (TYPE_PRECISION (long_integer_type_node) != 32
+ || POINTER_SIZE != 32
+ || TYPE_PRECISION (integer_type_node) != 32)
+ return MATCH_YES;
+ }
+ else
+ return MATCH_ERROR;
+}
+
if (gfc_vectorized_builtins == NULL)
gfc_vectorized_builtins = new hash_map ();
diff --git a/gcc/testsuite/gfortran.dg/simd-builtins-7.f90 b/gcc/testsuite/gfortran.dg/simd-builtins-7.f90
new file mode 100644
index 000..6445733d288
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/simd-builtins-7.f90
@@ -0,0 +1,21 @@
+! { dg-do compile { target { i?86-*-linux* x86_64-*-linux* } } }
+! { dg-additional-options "-msse2 -mno-avx -nostdinc -Ofast -fpre-include=simd-builtins-7.h -fdump-tree-optimized" }
+
+program test_overloaded_intrinsic
+ real(4) :: x4(3200), y4(3200)
+ real(8) :: x8(3200), y8(3200)
+
+ ! this should be using simd clone
+ y4 = sin(x4)
+ print *, y4
+
+ ! this should not be using simd clone
+ y4 = sin(x8)
+ print *, y8
+end
+
+! { dg-final { scan-tree-dump "sinf.simdclone" "optimized" { target ia32 } } } */
+! { dg-final { scan-tree-dump-not "sin.simdclone" "optimized" { target ia32 } } } */
+
+! { dg-final { scan-tree-dump "sin.simdclone" "optimized" { target lp64} } } */
+! { dg-final { scan-tree-dump-not "sinf.simdclone" "optimized" { target lp64 } } } */
diff --git a/gcc/testsuite/gfortran.dg/simd-builtins-7.h
Re: Fortran vector math header
On Mon, Jan 21, 2019 at 08:47:42AM +0100, Martin Liška wrote:
> diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
> index 3314e176881..5c7c062574b 100644
> --- a/gcc/fortran/decl.c
> +++ b/gcc/fortran/decl.c
> @@ -11351,6 +11351,7 @@ match
> gfc_match_gcc_builtin (void)
> {
>char builtin[GFC_MAX_SYMBOL_LEN + 1];
> + char target[GFC_MAX_SYMBOL_LEN + 1];
>
>if (gfc_match (" ( %n ) attributes simd", builtin) != MATCH_YES)
> return MATCH_ERROR;
> @@ -11361,6 +11362,24 @@ gfc_match_gcc_builtin (void)
>else if (gfc_match (" ( inbranch ) ") == MATCH_YES)
> clause = SIMD_INBRANCH;
>
> + if (gfc_match (" if ( %n ) ", target) == MATCH_YES)
> +{
> + unsigned HOST_WIDE_INT size_of_long
> + = tree_to_uhwi (TYPE_SIZE_UNIT (long_integer_type_node));
> + if (strcmp (target, "lp64") == 0)
> + {
> + if (size_of_long != 8)
> + return MATCH_YES;
__LP64__ macro is defined if:
if (TYPE_PRECISION (long_integer_type_node) == 64
&& POINTER_SIZE == 64
&& TYPE_PRECISION (integer_type_node) == 32)
All of these are middle-end types or target macros, so you should be able to
test that in the Fortran FE too.
> + }
> + else if (strcmp (target, "ilp32") == 0)
> + {
> + if (size_of_long != 4)
> + return MATCH_YES;
For this obviously as well.
> + }
> + else
> + return MATCH_ERROR;
> +}
> +
>if (gfc_vectorized_builtins == NULL)
> gfc_vectorized_builtins = new hash_map ();
Jakub
Re: Fortran vector math header
On 1/18/19 9:39 AM, Jakub Jelinek wrote:
> On Fri, Jan 18, 2019 at 09:18:33AM +0100, Martin Liška wrote:
>> What about something as simple as this:
>>
>> diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
>> index 3314e176881..2f2b965f97d 100644
>> --- a/gcc/fortran/decl.c
>> +++ b/gcc/fortran/decl.c
>> @@ -11361,6 +11361,11 @@ gfc_match_gcc_builtin (void)
>>else if (gfc_match (" ( inbranch ) ") == MATCH_YES)
>> clause = SIMD_INBRANCH;
>>
>> + /* Filter builtins defined only for 64-bit compilation mode. */
>> + if (gfc_match (" ( 64bit ) ") == MATCH_YES
>> + && tree_to_uhwi (TYPE_SIZE_UNIT (long_integer_type_node)) != 64)
>> +return MATCH_YES;
>> +
>>if (gfc_vectorized_builtins == NULL)
>> gfc_vectorized_builtins = new hash_map ();
>>
>> That would allow to write:
>> !GCC$ builtin (cos) attributes simd (notinbranch) (64bit)
>
> That feels too hacky to me.
> We could have
> !GCC$ builtin (cos) attributes simd (notinbranch) if('x86_64-linux-gnu')
> or similar if we can agree and get somehow canonical names of the multilib
> targets based on options, or just if('lp64'), if('ilp32'), or whatever other
> identifiers. The multiarch-style strings I'm afraid we have no way to
> propagate to f951 even on multiarch targets, if I understand it right, it is
> present there just in the form of substrings in the multi os directories.
> For some other strings, we'd need to come up with something that generates
> the strings for us, e.g. like config/*/*-d.c does for D have something
> similar for Fortran, and then we could use just x86_64, x32 and x86 or
> whatever else we choose (I guess the OS isn't that important, different OSes
> would have different headers). Even x86_64 vs. x32 vs. x86 shows that it
> isn't possible to differentiate multilibs just based on sizes (kinds) of C
> types, and even querying those is complicated because one needs to use the
> use iso_c_binding, only: c_ptr etc. to get those into the scope, which isn't
> something we want in these headers.
> In any case, glibc would need to agree with gfortran on these identifiers.
>
> Jakub
>
Hello.
I like the if('lp64'), if('ilp32') approach and I'm sending patch candidate for
that.
Would it be accepted by glibc folks?
Thanks,
Martin
>From d21476c2223e6e5d12a949b4e2ba26417e37852b Mon Sep 17 00:00:00 2001
From: marxin
Date: Mon, 21 Jan 2019 08:46:06 +0100
Subject: [PATCH] Support if statement in !GCC$ builtin directive.
---
gcc/fortran/decl.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 3314e176881..5c7c062574b 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -11351,6 +11351,7 @@ match
gfc_match_gcc_builtin (void)
{
char builtin[GFC_MAX_SYMBOL_LEN + 1];
+ char target[GFC_MAX_SYMBOL_LEN + 1];
if (gfc_match (" ( %n ) attributes simd", builtin) != MATCH_YES)
return MATCH_ERROR;
@@ -11361,6 +11362,24 @@ gfc_match_gcc_builtin (void)
else if (gfc_match (" ( inbranch ) ") == MATCH_YES)
clause = SIMD_INBRANCH;
+ if (gfc_match (" if ( %n ) ", target) == MATCH_YES)
+{
+ unsigned HOST_WIDE_INT size_of_long
+ = tree_to_uhwi (TYPE_SIZE_UNIT (long_integer_type_node));
+ if (strcmp (target, "lp64") == 0)
+ {
+ if (size_of_long != 8)
+ return MATCH_YES;
+ }
+ else if (strcmp (target, "ilp32") == 0)
+ {
+ if (size_of_long != 4)
+ return MATCH_YES;
+ }
+ else
+ return MATCH_ERROR;
+}
+
if (gfc_vectorized_builtins == NULL)
gfc_vectorized_builtins = new hash_map ();
--
2.20.1
Re: Fortran vector math header
On Fri, Jan 18, 2019 at 09:18:33AM +0100, Martin Liška wrote:
> What about something as simple as this:
>
> diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
> index 3314e176881..2f2b965f97d 100644
> --- a/gcc/fortran/decl.c
> +++ b/gcc/fortran/decl.c
> @@ -11361,6 +11361,11 @@ gfc_match_gcc_builtin (void)
>else if (gfc_match (" ( inbranch ) ") == MATCH_YES)
> clause = SIMD_INBRANCH;
>
> + /* Filter builtins defined only for 64-bit compilation mode. */
> + if (gfc_match (" ( 64bit ) ") == MATCH_YES
> + && tree_to_uhwi (TYPE_SIZE_UNIT (long_integer_type_node)) != 64)
> +return MATCH_YES;
> +
>if (gfc_vectorized_builtins == NULL)
> gfc_vectorized_builtins = new hash_map ();
>
> That would allow to write:
> !GCC$ builtin (cos) attributes simd (notinbranch) (64bit)
That feels too hacky to me.
We could have
!GCC$ builtin (cos) attributes simd (notinbranch) if('x86_64-linux-gnu')
or similar if we can agree and get somehow canonical names of the multilib
targets based on options, or just if('lp64'), if('ilp32'), or whatever other
identifiers. The multiarch-style strings I'm afraid we have no way to
propagate to f951 even on multiarch targets, if I understand it right, it is
present there just in the form of substrings in the multi os directories.
For some other strings, we'd need to come up with something that generates
the strings for us, e.g. like config/*/*-d.c does for D have something
similar for Fortran, and then we could use just x86_64, x32 and x86 or
whatever else we choose (I guess the OS isn't that important, different OSes
would have different headers). Even x86_64 vs. x32 vs. x86 shows that it
isn't possible to differentiate multilibs just based on sizes (kinds) of C
types, and even querying those is complicated because one needs to use the
use iso_c_binding, only: c_ptr etc. to get those into the scope, which isn't
something we want in these headers.
In any case, glibc would need to agree with gfortran on these identifiers.
Jakub
Re: Fortran vector math header
On 1/16/19 9:35 PM, Joseph Myers wrote:
> On Wed, 16 Jan 2019, Jakub Jelinek wrote:
>
>> Perhaps easier would be to add optional if clause to the !GCC$ builtin
>> with constant expression argument which if present and evaluates to .false.
>> would tell us to ignore the attribute. Or, add !GCC$ if/else/end if which
>> would act like preprocessing conditionals or something similar.
>> Not really sure one can query in Fortran what the multilib is some way (say
>> look at size of a pointer etc.).
>
> If something like that is done, I'd suggest doing it in a form which
> allows each multilib's information about glibc functions to go in a
> separate generated header (so having !GCC$ include or similar to include a
> per-multilib file, under appropriate conditionals). Otherwise you need to
> bring back logic in glibc to make a compiler building glibc for one
> multilib use appropriate -D and -U options to get its C headers to define
> things appropriately for another multilib, so that the all-multilib
> Fortran header can be generated in a single glibc build. (Like the old
> logic for generating bits/syscall.h that was removed in commits
> 2dba5ce7b8115d6a2789bf279892263621088e74 and
> ee17d4e99af9e49378217209d3708053ef148032.)
>
Hi.
What about something as simple as this:
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 3314e176881..2f2b965f97d 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -11361,6 +11361,11 @@ gfc_match_gcc_builtin (void)
else if (gfc_match (" ( inbranch ) ") == MATCH_YES)
clause = SIMD_INBRANCH;
+ /* Filter builtins defined only for 64-bit compilation mode. */
+ if (gfc_match (" ( 64bit ) ") == MATCH_YES
+ && tree_to_uhwi (TYPE_SIZE_UNIT (long_integer_type_node)) != 64)
+return MATCH_YES;
+
if (gfc_vectorized_builtins == NULL)
gfc_vectorized_builtins = new hash_map ();
That would allow to write:
!GCC$ builtin (cos) attributes simd (notinbranch) (64bit)
Thoughts?
Thanks,
Martin
Re: Fortran vector math header
On Wed, 16 Jan 2019, Jakub Jelinek wrote: > Perhaps easier would be to add optional if clause to the !GCC$ builtin > with constant expression argument which if present and evaluates to .false. > would tell us to ignore the attribute. Or, add !GCC$ if/else/end if which > would act like preprocessing conditionals or something similar. > Not really sure one can query in Fortran what the multilib is some way (say > look at size of a pointer etc.). If something like that is done, I'd suggest doing it in a form which allows each multilib's information about glibc functions to go in a separate generated header (so having !GCC$ include or similar to include a per-multilib file, under appropriate conditionals). Otherwise you need to bring back logic in glibc to make a compiler building glibc for one multilib use appropriate -D and -U options to get its C headers to define things appropriately for another multilib, so that the all-multilib Fortran header can be generated in a single glibc build. (Like the old logic for generating bits/syscall.h that was removed in commits 2dba5ce7b8115d6a2789bf279892263621088e74 and ee17d4e99af9e49378217209d3708053ef148032.) -- Joseph S. Myers jos...@codesourcery.com
Re: Fortran vector math header
On Wed, Jan 16, 2019 at 05:42:06PM +, Joseph Myers wrote: > On Wed, 16 Jan 2019, Joseph Myers wrote: > > > On Wed, 16 Jan 2019, Jakub Jelinek wrote: > > > > > In normal C headers, we can #if __WORDSIZE == 32 or __SIZEOF_LONG__ == 4 > > > or > > > defined(__ILP64__) and similar, but in these headers we can't, as no > > > preprocessing is happening. > > > > (With such preprocessing, the mechanism glibc uses for gnu/stubs.h and > > gnu/lib-names.h could be used.) > > And I guess this leads to the question: > > Since the Fortran front end has preprocessing support, could it be made to > run the preprocessor on such preincluded headers? If it could, the driver > code could be changed to avoid looking in multilib locations for this > header, and that glibc machinery could be used. (It would be necessary to > resolve the FIXME in fortran/cpp.c so that architecture-specific macros > get predefined; __x86_64__, __LP64__ and __ILP32__ all need to be properly > defined or not defined for this mechanism to work for x86.) Well, it has, but extremely inefficient one. Basically, it preprocesses everything into a temporary file and then parses that temporary file instead of the original input. Doing such preprocessing for this tiny header file that is now going to be included in every fortran compilation would slow it down, require the driver to tell where to create the temporary file etc. Perhaps easier would be to add optional if clause to the !GCC$ builtin with constant expression argument which if present and evaluates to .false. would tell us to ignore the attribute. Or, add !GCC$ if/else/end if which would act like preprocessing conditionals or something similar. Not really sure one can query in Fortran what the multilib is some way (say look at size of a pointer etc.). Jakub
Re: Fortran vector math header
On Wed, 16 Jan 2019, Joseph Myers wrote: > On Wed, 16 Jan 2019, Jakub Jelinek wrote: > > > In normal C headers, we can #if __WORDSIZE == 32 or __SIZEOF_LONG__ == 4 or > > defined(__ILP64__) and similar, but in these headers we can't, as no > > preprocessing is happening. > > (With such preprocessing, the mechanism glibc uses for gnu/stubs.h and > gnu/lib-names.h could be used.) And I guess this leads to the question: Since the Fortran front end has preprocessing support, could it be made to run the preprocessor on such preincluded headers? If it could, the driver code could be changed to avoid looking in multilib locations for this header, and that glibc machinery could be used. (It would be necessary to resolve the FIXME in fortran/cpp.c so that architecture-specific macros get predefined; __x86_64__, __LP64__ and __ILP32__ all need to be properly defined or not defined for this mechanism to work for x86.) -- Joseph S. Myers jos...@codesourcery.com
Re: Fortran vector math header
On Wed, 16 Jan 2019, Jakub Jelinek wrote:
> In normal C headers, we can #if __WORDSIZE == 32 or __SIZEOF_LONG__ == 4 or
> defined(__ILP64__) and similar, but in these headers we can't, as no
> preprocessing is happening.
(With such preprocessing, the mechanism glibc uses for gnu/stubs.h and
gnu/lib-names.h could be used.)
> So, I think we need more multi-arch like setup for these locations, say
> /usr/include/finclude/{,/}/
> or similar which would be less ambiguous.
My understanding from the previous discussions is that with
--enable-multiarch, these files will (or should) use multiarch names - but
we don't have any support for using multiarch names for only some files in
a toolchain. (Which leads to suggestions like this one for having a
target triplet in there which isn't actually a canonical multiarch name.)
--
Joseph S. Myers
jos...@codesourcery.com
Re: Fortran vector math header
On Wed, 16 Jan 2019, Martin Liška wrote: > Now we should add the header file into glibc, I kicked a discussion here: > https://sourceware.org/ml/libc-help/2018-11/msg00015.html libc-help is for user questions. It's not suitable for glibc development discussions. -- Joseph S. Myers jos...@codesourcery.com
Re: Fortran vector math header
On Wed, Jan 16, 2019 at 10:42:13AM +0100, Martin Liška wrote:
> On 1/15/19 6:45 PM, Joseph Myers wrote:
> > On Mon, 14 Jan 2019, Martin Liška wrote:
> >
> >> Thanks for review, fixed that in updated version of the patch.
> >>
> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >>
> >> Ready to be installed?
> >
> > This patch is OK.
> >
>
> Great, thank you very much for advises you provided.
>
> Now we should add the header file into glibc, I kicked a discussion here:
> https://sourceware.org/ml/libc-help/2018-11/msg00015.html
>
> Joseph: will you please help with a patch for glibc?
>
> I'm also planning to document the header file in gfortran so that other
> compiler can leverage that.
I'd like to raise a concern I have.
On IRC you've mentioned you'd like to see that header in
/usr/include/finclude/{,32/}math-vector-fortran.h
(with 32/ header being empty) which looks problematic to me.
The 32/ in there is the compiler's multilib suffix, which is though
dependent on how exactly one configures the compiler.
On x86_64-linux-gnu, one can either build (native) x86_64-pc-linux-gnu compiler
that defaults to -m64, supports -m32 and uses . and 32/ multilibs for those,
or one can build also native i686-pc-linux-gnu compiler (ideally with 32-bit
binutils too) that only supports -m32 and uses . multilib for the 32-bit.
If glibc installs the above, then if one uses the 32-bit i686-linux
gfortran, it will misbehave, as it will think glibc.i686 provides the
math-vector.h entrypoints when it doesn't.
Right now no other target supports them, but if say powerpc64-linux got
them, there one can build powerpc-linux gcc that supports -m32 and -m64
with . and 64/ multilib subdirs, or powerpc64-linux gcc that supports
-m64 and -m32 with . and 32/ multilib subdirs. So, what would
/usr/include/finclude/math-vector-fortran.h stand for in that case (again,
assuming the entry points are implemented in one of the multilibs only).
In normal C headers, we can #if __WORDSIZE == 32 or __SIZEOF_LONG__ == 4 or
defined(__ILP64__) and similar, but in these headers we can't, as no
preprocessing is happening.
So, I think we need more multi-arch like setup for these locations, say
/usr/include/finclude/{,/}/
or similar which would be less ambiguous.
Jakub
