diff mbox series

Fix location where math-vector-fortran.h is installed.

Message ID 44cd0edb-2ab3-2908-8adb-f2e06859ecfe@suse.cz
State New
Headers show
Series Fix location where math-vector-fortran.h is installed. | expand

Commit Message

Martin Liška Feb. 26, 2019, 5:56 p.m. UTC
Hi.

This is follow up patch where I fix location where the header
is installed. I made a type as I was copying & pasting.

I also built glibc package in openSUSE and verified a recent GCC
can properly find the header.

Ready for trunk?
Thanks,
Martin

Comments

Joseph Myers Feb. 26, 2019, 6:05 p.m. UTC | #1
On Tue, 26 Feb 2019, Martin Liška wrote:

> Hi.
> 
> This is follow up patch where I fix location where the header
> is installed. I made a type as I was copying & pasting.

It's not clear to me that we want to add a top-level finclude/ directory 
in the glibc source tree.  Would math/finclude/ work just as well as a 
source tree location for this file?
Martin Liška Feb. 26, 2019, 6:30 p.m. UTC | #2
On 2/26/19 7:05 PM, Joseph Myers wrote:
> On Tue, 26 Feb 2019, Martin Liška wrote:
> 
>> Hi.
>>
>> This is follow up patch where I fix location where the header
>> is installed. I made a type as I was copying & pasting.
> 
> It's not clear to me that we want to add a top-level finclude/ directory 
> in the glibc source tree.  Would math/finclude/ work just as well as a 
> source tree location for this file?
> 

Yes, I can confirm math/finclude work as well.

May I install the patch?
Martin
From cb267faade1b6558e895b496294cd8783236be3f Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 26 Feb 2019 18:17:36 +0100
Subject: [PATCH] Fix location where math-vector-fortran.h is installed.

ChangeLog:

2019-02-26  Martin Liska  <mliska@suse.cz>

	* math/Makefile: Change location where math-vector-fortran.h is
	installed.
	* finclude/math-vector-fortran.h: Move from bits/math-vector-fortran.h.
	* sysdeps/x86/fpu/finclude/math-vector-fortran.h: Move
	from sysdeps/x86/fpu/bits/math-vector-fortran.h.
---
 math/Makefile                                            | 2 +-
 {bits => math/finclude}/math-vector-fortran.h            | 0
 sysdeps/x86/fpu/{bits => finclude}/math-vector-fortran.h | 0
 3 files changed, 1 insertion(+), 1 deletion(-)
 rename {bits => math/finclude}/math-vector-fortran.h (100%)
 rename sysdeps/x86/fpu/{bits => finclude}/math-vector-fortran.h (100%)

diff --git a/math/Makefile b/math/Makefile
index fc4191089d..cb4eaec6a9 100644
--- a/math/Makefile
+++ b/math/Makefile
@@ -26,7 +26,7 @@ headers		:= math.h bits/mathcalls.h bits/mathinline.h \
 		   fpu_control.h complex.h bits/cmathcalls.h fenv.h \
 		   bits/fenv.h bits/fenvinline.h bits/mathdef.h tgmath.h \
 		   bits/math-finite.h bits/math-vector.h \
-		   bits/math-vector-fortran.h \
+		   finclude/math-vector-fortran.h \
 		   bits/libm-simd-decl-stubs.h bits/iscanonical.h \
 		   bits/flt-eval-method.h bits/fp-fast.h bits/fp-logb.h \
 		   bits/long-double.h bits/mathcalls-helper-functions.h \
diff --git a/bits/math-vector-fortran.h b/math/finclude/math-vector-fortran.h
similarity index 100%
rename from bits/math-vector-fortran.h
rename to math/finclude/math-vector-fortran.h
diff --git a/sysdeps/x86/fpu/bits/math-vector-fortran.h b/sysdeps/x86/fpu/finclude/math-vector-fortran.h
similarity index 100%
rename from sysdeps/x86/fpu/bits/math-vector-fortran.h
rename to sysdeps/x86/fpu/finclude/math-vector-fortran.h
Florian Weimer Feb. 26, 2019, 9:16 p.m. UTC | #3
* Martin Liška:

> On 2/26/19 7:05 PM, Joseph Myers wrote:
>> On Tue, 26 Feb 2019, Martin Liška wrote:
>> 
>>> Hi.
>>>
>>> This is follow up patch where I fix location where the header
>>> is installed. I made a type as I was copying & pasting.
>> 
>> It's not clear to me that we want to add a top-level finclude/ directory 
>> in the glibc source tree.  Would math/finclude/ work just as well as a 
>> source tree location for this file?
>> 
>
> Yes, I can confirm math/finclude work as well.
>
> May I install the patch?

This causes test suite failures for me:

math/check-installed-headers-c.out:

In file included from /tmp/cih_test_C67G81.c:8:
../sysdeps/x86/fpu/finclude/math-vector-fortran.h:1:1: error: expected identifie
r or ‘(’ before ‘!’ token
 ! Platform-specific declarations of SIMD math functions for Fortran. -*- f90 -*
-
 ^

math/check-wrapper-headers.out:

error: missing wrapper header include/finclude/math-vector-fortran.h for math/finclude/math-vector-fortran.h

I think we need to whitelist this in multiple places.  It is unfortunate
that Fortran headers use .h like C headers.

Is the finclude/ prefix standardized?

Thanks,
Florian
Zack Weinberg Feb. 26, 2019, 9:41 p.m. UTC | #4
On Tue, Feb 26, 2019 at 4:16 PM Florian Weimer <fweimer@redhat.com> wrote:
> * Martin Liška:
> > On 2/26/19 7:05 PM, Joseph Myers wrote:
> >> On Tue, 26 Feb 2019, Martin Liška wrote:
> >>
> >>> Hi.
> >>>
> >>> This is follow up patch where I fix location where the header
> >>> is installed. I made a type as I was copying & pasting.

Do I understand correctly that the desired installation location is
$(prefix)/include/finclude/math-vector-fortran.h ?

> This causes test suite failures for me:
>
> math/check-installed-headers-c.out:
>
> In file included from /tmp/cih_test_C67G81.c:8:
> ../sysdeps/x86/fpu/finclude/math-vector-fortran.h:1:1: error: expected identifie
> r or ‘(’ before ‘!’ token
>  ! Platform-specific declarations of SIMD math functions for Fortran. -*- f90 -*

Moving the file out of bits/ caused it to be newly subject to these tests.

If $(prefix)/include/finclude/ is intended to contain nothing but
Fortran headers, we could add finclude to the list of subdirectories
to skip in scripts/check*headers* ...

zw
Martin Liška Feb. 27, 2019, 8:09 a.m. UTC | #5
On 2/26/19 10:41 PM, Zack Weinberg wrote:
> On Tue, Feb 26, 2019 at 4:16 PM Florian Weimer <fweimer@redhat.com> wrote:
>> * Martin Liška:
>>> On 2/26/19 7:05 PM, Joseph Myers wrote:
>>>> On Tue, 26 Feb 2019, Martin Liška wrote:
>>>>
>>>>> Hi.
>>>>>
>>>>> This is follow up patch where I fix location where the header
>>>>> is installed. I made a type as I was copying & pasting.
> 
> Do I understand correctly that the desired installation location is
> $(prefix)/include/finclude/math-vector-fortran.h ?

Yes.

> 
>> This causes test suite failures for me:
>>
>> math/check-installed-headers-c.out:
>>
>> In file included from /tmp/cih_test_C67G81.c:8:
>> ../sysdeps/x86/fpu/finclude/math-vector-fortran.h:1:1: error: expected identifie
>> r or ‘(’ before ‘!’ token
>>  ! Platform-specific declarations of SIMD math functions for Fortran. -*- f90 -*

I skipped that header explicitly.

> 
> Moving the file out of bits/ caused it to be newly subject to these tests.
> 
> If $(prefix)/include/finclude/ is intended to contain nothing but
> Fortran headers, we could add finclude to the list of subdirectories
> to skip in scripts/check*headers* ...
> 
> zw
> 

And for the check-wrapper-headers.py, I read a header file and skip files
with 'f90' file type.

Updated patch is attached.

Martin
From 597b7ec78b23f8bd76a4d8acfeaee265f06f2a1b Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 26 Feb 2019 18:17:36 +0100
Subject: [PATCH] Fix location where math-vector-fortran.h is installed.

ChangeLog:

2019-02-26  Martin Liska  <mliska@suse.cz>

	* math/Makefile: Change location where math-vector-fortran.h is
	installed.
	* finclude/math-vector-fortran.h: Move from bits/math-vector-fortran.h.
	* sysdeps/x86/fpu/finclude/math-vector-fortran.h: Move
	from sysdeps/x86/fpu/bits/math-vector-fortran.h.
	* scripts/check-installed-headers.sh: Skip Fortran header file.
	* scripts/check-wrapper-headers.py: Filter out f90 files.
---
 math/Makefile                                            | 2 +-
 {bits => math/finclude}/math-vector-fortran.h            | 0
 scripts/check-installed-headers.sh                       | 4 ++++
 scripts/check-wrapper-headers.py                         | 4 ++++
 sysdeps/x86/fpu/{bits => finclude}/math-vector-fortran.h | 0
 5 files changed, 9 insertions(+), 1 deletion(-)
 rename {bits => math/finclude}/math-vector-fortran.h (100%)
 rename sysdeps/x86/fpu/{bits => finclude}/math-vector-fortran.h (100%)

diff --git a/math/Makefile b/math/Makefile
index fc4191089d..cb4eaec6a9 100644
--- a/math/Makefile
+++ b/math/Makefile
@@ -26,7 +26,7 @@ headers		:= math.h bits/mathcalls.h bits/mathinline.h \
 		   fpu_control.h complex.h bits/cmathcalls.h fenv.h \
 		   bits/fenv.h bits/fenvinline.h bits/mathdef.h tgmath.h \
 		   bits/math-finite.h bits/math-vector.h \
-		   bits/math-vector-fortran.h \
+		   finclude/math-vector-fortran.h \
 		   bits/libm-simd-decl-stubs.h bits/iscanonical.h \
 		   bits/flt-eval-method.h bits/fp-fast.h bits/fp-logb.h \
 		   bits/long-double.h bits/mathcalls-helper-functions.h \
diff --git a/bits/math-vector-fortran.h b/math/finclude/math-vector-fortran.h
similarity index 100%
rename from bits/math-vector-fortran.h
rename to math/finclude/math-vector-fortran.h
diff --git a/scripts/check-installed-headers.sh b/scripts/check-installed-headers.sh
index 8e7beffd82..937ed969ec 100644
--- a/scripts/check-installed-headers.sh
+++ b/scripts/check-installed-headers.sh
@@ -84,6 +84,10 @@ for header in "$@"; do
         (sys/elf.h)
             continue;;
 
+        # Skip Fortran header
+        (finclude/math-vector-fortran.h)
+            continue;;
+
 	# sys/sysctl.h is unsupported for x32.
 	(sys/sysctl.h)
             case "$is_x32" in
diff --git a/scripts/check-wrapper-headers.py b/scripts/check-wrapper-headers.py
index 094faa3ced..5c982c3eb4 100644
--- a/scripts/check-wrapper-headers.py
+++ b/scripts/check-wrapper-headers.py
@@ -75,6 +75,10 @@ def check_headers(args):
 
         is_nonsysdep_header = os.access(header, os.R_OK)
         if is_nonsysdep_header:
+            # Skip Fortran header files
+            if '-*- f90 -*-' in open(header).readlines()[0]:
+                continue
+
             include_path = os.path.join(args.root, INCLUDE, header)
             if not os.access(include_path, os.R_OK):
                 print('error: missing wrapper header {} for {}'.format(
diff --git a/sysdeps/x86/fpu/bits/math-vector-fortran.h b/sysdeps/x86/fpu/finclude/math-vector-fortran.h
similarity index 100%
rename from sysdeps/x86/fpu/bits/math-vector-fortran.h
rename to sysdeps/x86/fpu/finclude/math-vector-fortran.h
Florian Weimer Feb. 27, 2019, 10:12 a.m. UTC | #6
* Martin Liška:

>          is_nonsysdep_header = os.access(header, os.R_OK)
>          if is_nonsysdep_header:
> +            # Skip Fortran header files
> +            if '-*- f90 -*-' in open(header).readlines()[0]:
> +                continue

Please use “with” to close the file promptly, and do not read the entire
file, like this (untested):

    # Skip Fortran header files.
    with open(header) as inp:
        if '-*- f90 -*-' in next(inp):
            continue

Thanks,
Florian
Martin Liška Feb. 27, 2019, 10:36 a.m. UTC | #7
On 2/27/19 11:12 AM, Florian Weimer wrote:
> * Martin Liška:
> 
>>          is_nonsysdep_header = os.access(header, os.R_OK)
>>          if is_nonsysdep_header:
>> +            # Skip Fortran header files
>> +            if '-*- f90 -*-' in open(header).readlines()[0]:
>> +                continue
> 
> Please use “with” to close the file promptly, and do not read the entire
> file, like this (untested):
> 
>     # Skip Fortran header files.
>     with open(header) as inp:
>         if '-*- f90 -*-' in next(inp):
>             continue

Good point. Is the patch ready to be installed now?

Thanks,
Martin

> 
> Thanks,
> Florian
>
From 5e786337c48dbf33453a634725e2e95b10c73b62 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 26 Feb 2019 18:17:36 +0100
Subject: [PATCH] Fix location where math-vector-fortran.h is installed.

ChangeLog:

2019-02-26  Martin Liska  <mliska@suse.cz>

	* math/Makefile: Change location where math-vector-fortran.h is
	installed.
	* finclude/math-vector-fortran.h: Move from bits/math-vector-fortran.h.
	* sysdeps/x86/fpu/finclude/math-vector-fortran.h: Move
	from sysdeps/x86/fpu/bits/math-vector-fortran.h.
	* scripts/check-installed-headers.sh: Skip Fortran header file.
	* scripts/check-wrapper-headers.py: Filter out f90 files.
---
 math/Makefile                                            | 2 +-
 {bits => math/finclude}/math-vector-fortran.h            | 0
 scripts/check-installed-headers.sh                       | 4 ++++
 scripts/check-wrapper-headers.py                         | 5 +++++
 sysdeps/x86/fpu/{bits => finclude}/math-vector-fortran.h | 0
 5 files changed, 10 insertions(+), 1 deletion(-)
 rename {bits => math/finclude}/math-vector-fortran.h (100%)
 rename sysdeps/x86/fpu/{bits => finclude}/math-vector-fortran.h (100%)

diff --git a/math/Makefile b/math/Makefile
index fc4191089d..cb4eaec6a9 100644
--- a/math/Makefile
+++ b/math/Makefile
@@ -26,7 +26,7 @@ headers		:= math.h bits/mathcalls.h bits/mathinline.h \
 		   fpu_control.h complex.h bits/cmathcalls.h fenv.h \
 		   bits/fenv.h bits/fenvinline.h bits/mathdef.h tgmath.h \
 		   bits/math-finite.h bits/math-vector.h \
-		   bits/math-vector-fortran.h \
+		   finclude/math-vector-fortran.h \
 		   bits/libm-simd-decl-stubs.h bits/iscanonical.h \
 		   bits/flt-eval-method.h bits/fp-fast.h bits/fp-logb.h \
 		   bits/long-double.h bits/mathcalls-helper-functions.h \
diff --git a/bits/math-vector-fortran.h b/math/finclude/math-vector-fortran.h
similarity index 100%
rename from bits/math-vector-fortran.h
rename to math/finclude/math-vector-fortran.h
diff --git a/scripts/check-installed-headers.sh b/scripts/check-installed-headers.sh
index 8e7beffd82..937ed969ec 100644
--- a/scripts/check-installed-headers.sh
+++ b/scripts/check-installed-headers.sh
@@ -84,6 +84,10 @@ for header in "$@"; do
         (sys/elf.h)
             continue;;
 
+        # Skip Fortran header
+        (finclude/math-vector-fortran.h)
+            continue;;
+
 	# sys/sysctl.h is unsupported for x32.
 	(sys/sysctl.h)
             case "$is_x32" in
diff --git a/scripts/check-wrapper-headers.py b/scripts/check-wrapper-headers.py
index 094faa3ced..6d9a89bc98 100644
--- a/scripts/check-wrapper-headers.py
+++ b/scripts/check-wrapper-headers.py
@@ -75,6 +75,11 @@ def check_headers(args):
 
         is_nonsysdep_header = os.access(header, os.R_OK)
         if is_nonsysdep_header:
+            # Skip Fortran header files
+            with open(header) as inp:
+                if '-*- f90 -*-' in next(inp):
+                    continue
+
             include_path = os.path.join(args.root, INCLUDE, header)
             if not os.access(include_path, os.R_OK):
                 print('error: missing wrapper header {} for {}'.format(
diff --git a/sysdeps/x86/fpu/bits/math-vector-fortran.h b/sysdeps/x86/fpu/finclude/math-vector-fortran.h
similarity index 100%
rename from sysdeps/x86/fpu/bits/math-vector-fortran.h
rename to sysdeps/x86/fpu/finclude/math-vector-fortran.h
Andreas Schwab Feb. 27, 2019, 11:30 a.m. UTC | #8
On Feb 27 2019, Martin Liška <mliska@suse.cz> wrote:

> 2019-02-26  Martin Liska  <mliska@suse.cz>
>
> 	* math/Makefile: Change location where math-vector-fortran.h is
> 	installed.
> 	* finclude/math-vector-fortran.h: Move from bits/math-vector-fortran.h.

That file resides in math/.

Andreas.
Martin Liška Feb. 27, 2019, 11:53 a.m. UTC | #9
On 2/27/19 12:30 PM, Andreas Schwab wrote:
> On Feb 27 2019, Martin Liška <mliska@suse.cz> wrote:
> 
>> 2019-02-26  Martin Liska  <mliska@suse.cz>
>>
>> 	* math/Makefile: Change location where math-vector-fortran.h is
>> 	installed.
>> 	* finclude/math-vector-fortran.h: Move from bits/math-vector-fortran.h.
> 
> That file resides in math/.
> 
> Andreas.
> 

Sure.

Ready for trunk?
Thanks,
Martin
From bad39d298a37b3411ccc323ffe7a94d4891fe2f0 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 26 Feb 2019 18:17:36 +0100
Subject: [PATCH] Fix location where math-vector-fortran.h is installed.

ChangeLog:

2019-02-26  Martin Liska  <mliska@suse.cz>

	* math/Makefile: Change location where math-vector-fortran.h is
	installed.
	* math/finclude/math-vector-fortran.h: Move from bits/math-vector-fortran.h.
	* sysdeps/x86/fpu/finclude/math-vector-fortran.h: Move
	from sysdeps/x86/fpu/bits/math-vector-fortran.h.
	* scripts/check-installed-headers.sh: Skip Fortran header file.
	* scripts/check-wrapper-headers.py: Filter out f90 files.
---
 math/Makefile                                            | 2 +-
 {bits => math/finclude}/math-vector-fortran.h            | 0
 scripts/check-installed-headers.sh                       | 4 ++++
 scripts/check-wrapper-headers.py                         | 5 +++++
 sysdeps/x86/fpu/{bits => finclude}/math-vector-fortran.h | 0
 5 files changed, 10 insertions(+), 1 deletion(-)
 rename {bits => math/finclude}/math-vector-fortran.h (100%)
 rename sysdeps/x86/fpu/{bits => finclude}/math-vector-fortran.h (100%)

diff --git a/math/Makefile b/math/Makefile
index fc4191089d..cb4eaec6a9 100644
--- a/math/Makefile
+++ b/math/Makefile
@@ -26,7 +26,7 @@ headers		:= math.h bits/mathcalls.h bits/mathinline.h \
 		   fpu_control.h complex.h bits/cmathcalls.h fenv.h \
 		   bits/fenv.h bits/fenvinline.h bits/mathdef.h tgmath.h \
 		   bits/math-finite.h bits/math-vector.h \
-		   bits/math-vector-fortran.h \
+		   finclude/math-vector-fortran.h \
 		   bits/libm-simd-decl-stubs.h bits/iscanonical.h \
 		   bits/flt-eval-method.h bits/fp-fast.h bits/fp-logb.h \
 		   bits/long-double.h bits/mathcalls-helper-functions.h \
diff --git a/bits/math-vector-fortran.h b/math/finclude/math-vector-fortran.h
similarity index 100%
rename from bits/math-vector-fortran.h
rename to math/finclude/math-vector-fortran.h
diff --git a/scripts/check-installed-headers.sh b/scripts/check-installed-headers.sh
index 8e7beffd82..937ed969ec 100644
--- a/scripts/check-installed-headers.sh
+++ b/scripts/check-installed-headers.sh
@@ -84,6 +84,10 @@ for header in "$@"; do
         (sys/elf.h)
             continue;;
 
+        # Skip Fortran header
+        (finclude/math-vector-fortran.h)
+            continue;;
+
 	# sys/sysctl.h is unsupported for x32.
 	(sys/sysctl.h)
             case "$is_x32" in
diff --git a/scripts/check-wrapper-headers.py b/scripts/check-wrapper-headers.py
index 094faa3ced..6d9a89bc98 100644
--- a/scripts/check-wrapper-headers.py
+++ b/scripts/check-wrapper-headers.py
@@ -75,6 +75,11 @@ def check_headers(args):
 
         is_nonsysdep_header = os.access(header, os.R_OK)
         if is_nonsysdep_header:
+            # Skip Fortran header files
+            with open(header) as inp:
+                if '-*- f90 -*-' in next(inp):
+                    continue
+
             include_path = os.path.join(args.root, INCLUDE, header)
             if not os.access(include_path, os.R_OK):
                 print('error: missing wrapper header {} for {}'.format(
diff --git a/sysdeps/x86/fpu/bits/math-vector-fortran.h b/sysdeps/x86/fpu/finclude/math-vector-fortran.h
similarity index 100%
rename from sysdeps/x86/fpu/bits/math-vector-fortran.h
rename to sysdeps/x86/fpu/finclude/math-vector-fortran.h
Zack Weinberg Feb. 27, 2019, 3 p.m. UTC | #10
On Wed, Feb 27, 2019 at 3:09 AM Martin Liška <mliska@suse.cz> wrote:
> On 2/26/19 10:41 PM, Zack Weinberg wrote:
> > On Tue, Feb 26, 2019 at 4:16 PM Florian Weimer <fweimer@redhat.com> wrote:
> >> * Martin Liška:
> >>> On 2/26/19 7:05 PM, Joseph Myers wrote:
> >>>> On Tue, 26 Feb 2019, Martin Liška wrote:
> >>>>
> >>>>> Hi.
> >>>>>
> >>>>> This is follow up patch where I fix location where the header
> >>>>> is installed. I made a type as I was copying & pasting.
> >
> > Do I understand correctly that the desired installation location is
> > $(prefix)/include/finclude/math-vector-fortran.h ?
>
> Yes.

Will there ever be C headers in $(prefix)/include/finclude ?

> >> In file included from /tmp/cih_test_C67G81.c:8:
> >> ../sysdeps/x86/fpu/finclude/math-vector-fortran.h:1:1: error: expected identifie
> >> r or ‘(’ before ‘!’ token
> >>  ! Platform-specific declarations of SIMD math functions for Fortran. -*- f90 -*
>
> I skipped that header explicitly.
...
> +        # Skip Fortran header
> +        (finclude/math-vector-fortran.h)
> +            continue;;

To put the above question another way, is there a reason why this shouldn't be

+        # Skip Fortran headers
+        (finclude/*)
+            continue;;

?

> And for the check-wrapper-headers.py, I read a header file and skip files
> with 'f90' file type.

If you're going to do it this way, I recommend you copy the more
general Emacs modeline parser from
https://sourceware.org/ml/libc-alpha/2019-02/msg00539.html ... but if
we can just skip finclude/* that might be a better approach for both
scripts.

zw
Martin Liška Feb. 28, 2019, 1:18 p.m. UTC | #11
On 2/27/19 4:00 PM, Zack Weinberg wrote:
> On Wed, Feb 27, 2019 at 3:09 AM Martin Liška <mliska@suse.cz> wrote:
>> On 2/26/19 10:41 PM, Zack Weinberg wrote:
>>> On Tue, Feb 26, 2019 at 4:16 PM Florian Weimer <fweimer@redhat.com> wrote:
>>>> * Martin Liška:
>>>>> On 2/26/19 7:05 PM, Joseph Myers wrote:
>>>>>> On Tue, 26 Feb 2019, Martin Liška wrote:
>>>>>>
>>>>>>> Hi.
>>>>>>>
>>>>>>> This is follow up patch where I fix location where the header
>>>>>>> is installed. I made a type as I was copying & pasting.
>>>
>>> Do I understand correctly that the desired installation location is
>>> $(prefix)/include/finclude/math-vector-fortran.h ?
>>
>> Yes.
> 
> Will there ever be C headers in $(prefix)/include/finclude ?

Hello.

No, only Fortran header files.

> 
>>>> In file included from /tmp/cih_test_C67G81.c:8:
>>>> ../sysdeps/x86/fpu/finclude/math-vector-fortran.h:1:1: error: expected identifie
>>>> r or ‘(’ before ‘!’ token
>>>>  ! Platform-specific declarations of SIMD math functions for Fortran. -*- f90 -*
>>
>> I skipped that header explicitly.
> ...
>> +        # Skip Fortran header
>> +        (finclude/math-vector-fortran.h)
>> +            continue;;
> 
> To put the above question another way, is there a reason why this shouldn't be
> 
> +        # Skip Fortran headers
> +        (finclude/*)
> +            continue;;
> 
> ?

Yes.

> 
>> And for the check-wrapper-headers.py, I read a header file and skip files
>> with 'f90' file type.
> 
> If you're going to do it this way, I recommend you copy the more
> general Emacs modeline parser from
> https://sourceware.org/ml/libc-alpha/2019-02/msg00539.html ... but if
> we can just skip finclude/* that might be a better approach for both
> scripts.

Agree, it would be easier.

Martin

> 
> zw
>
From b52fa57a42a335d50f03cfe4addd9d92e31851b6 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 26 Feb 2019 18:17:36 +0100
Subject: [PATCH] Fix location where math-vector-fortran.h is installed.

ChangeLog:

2019-02-26  Martin Liska  <mliska@suse.cz>

	* math/Makefile: Change location where math-vector-fortran.h is
	installed.
	* math/finclude/math-vector-fortran.h: Move from bits/math-vector-fortran.h.
	* sysdeps/x86/fpu/finclude/math-vector-fortran.h: Move
	from sysdeps/x86/fpu/bits/math-vector-fortran.h.
	* scripts/check-installed-headers.sh: Skip Fortran header files.
	* scripts/check-wrapper-headers.py: Likewise.
---
 math/Makefile                                            | 2 +-
 {bits => math/finclude}/math-vector-fortran.h            | 2 +-
 scripts/check-installed-headers.sh                       | 4 ++++
 scripts/check-wrapper-headers.py                         | 4 ++++
 sysdeps/x86/fpu/{bits => finclude}/math-vector-fortran.h | 2 +-
 5 files changed, 11 insertions(+), 3 deletions(-)
 rename {bits => math/finclude}/math-vector-fortran.h (98%)
 rename sysdeps/x86/fpu/{bits => finclude}/math-vector-fortran.h (99%)

diff --git a/math/Makefile b/math/Makefile
index fc4191089d..cb4eaec6a9 100644
--- a/math/Makefile
+++ b/math/Makefile
@@ -26,7 +26,7 @@ headers		:= math.h bits/mathcalls.h bits/mathinline.h \
 		   fpu_control.h complex.h bits/cmathcalls.h fenv.h \
 		   bits/fenv.h bits/fenvinline.h bits/mathdef.h tgmath.h \
 		   bits/math-finite.h bits/math-vector.h \
-		   bits/math-vector-fortran.h \
+		   finclude/math-vector-fortran.h \
 		   bits/libm-simd-decl-stubs.h bits/iscanonical.h \
 		   bits/flt-eval-method.h bits/fp-fast.h bits/fp-logb.h \
 		   bits/long-double.h bits/mathcalls-helper-functions.h \
diff --git a/bits/math-vector-fortran.h b/math/finclude/math-vector-fortran.h
similarity index 98%
rename from bits/math-vector-fortran.h
rename to math/finclude/math-vector-fortran.h
index 7c1e095094..66d467c22d 100644
--- a/bits/math-vector-fortran.h
+++ b/math/finclude/math-vector-fortran.h
@@ -1,4 +1,4 @@
-! Platform-specific declarations of SIMD math functions for Fortran. -*- f90 -*-
+! Platform-specific declarations of SIMD math functions for Fortran.
 !   Copyright (C) 2019 Free Software Foundation, Inc.
 !   This file is part of the GNU C Library.
 !
diff --git a/scripts/check-installed-headers.sh b/scripts/check-installed-headers.sh
index 8e7beffd82..e90bdc389f 100644
--- a/scripts/check-installed-headers.sh
+++ b/scripts/check-installed-headers.sh
@@ -84,6 +84,10 @@ for header in "$@"; do
         (sys/elf.h)
             continue;;
 
+        # Skip Fortran headers
+        (finclude/*)
+            continue;;
+
 	# sys/sysctl.h is unsupported for x32.
 	(sys/sysctl.h)
             case "$is_x32" in
diff --git a/scripts/check-wrapper-headers.py b/scripts/check-wrapper-headers.py
index 094faa3ced..fc3c25d7de 100644
--- a/scripts/check-wrapper-headers.py
+++ b/scripts/check-wrapper-headers.py
@@ -75,6 +75,10 @@ def check_headers(args):
 
         is_nonsysdep_header = os.access(header, os.R_OK)
         if is_nonsysdep_header:
+            # Skip Fortran header files
+            if 'finclude' in header:
+                continue
+
             include_path = os.path.join(args.root, INCLUDE, header)
             if not os.access(include_path, os.R_OK):
                 print('error: missing wrapper header {} for {}'.format(
diff --git a/sysdeps/x86/fpu/bits/math-vector-fortran.h b/sysdeps/x86/fpu/finclude/math-vector-fortran.h
similarity index 99%
rename from sysdeps/x86/fpu/bits/math-vector-fortran.h
rename to sysdeps/x86/fpu/finclude/math-vector-fortran.h
index 36051cc73e..4b69beed67 100644
--- a/sysdeps/x86/fpu/bits/math-vector-fortran.h
+++ b/sysdeps/x86/fpu/finclude/math-vector-fortran.h
@@ -1,4 +1,4 @@
-! Platform-specific declarations of SIMD math functions for Fortran. -*- f90 -*-
+! Platform-specific declarations of SIMD math functions for Fortran.
 !   Copyright (C) 2019 Free Software Foundation, Inc.
 !   This file is part of the GNU C Library.
 !
Zack Weinberg Feb. 28, 2019, 1:30 p.m. UTC | #12
Revised patch looks good to me except:

> -! Platform-specific declarations of SIMD math functions for Fortran. -*- f90 -*-
> +! Platform-specific declarations of SIMD math functions for Fortran.

Let's keep the -*- f90 -*- annotations, they will also be helpful for
future people editing these files.  For instance, on my computer, both
emacs and vim use the Fortran syntax highlighting rules for these
files when the annotation is present, and the C rules when it's
absent.

> +            # Skip Fortran header files
> +            if 'finclude' in header:
> +                continue

I think this conditional should be

    if header.startswith('finclude/') or '/finclude/' in header

so that it only applies to headers in the directory 'finclude', not
any hypothetical future headers that happen to have the string
'finclude' in their base names.  (I don't know why that would happen,
but if it ever did, I can see someone being really confused why it
wasn't getting tested, so some extra defensiveness now seems like a
good idea.)

zw
Florian Weimer Feb. 28, 2019, 1:32 p.m. UTC | #13
* Zack Weinberg:

> Revised patch looks good to me except:
>
>> -! Platform-specific declarations of SIMD math functions for Fortran. -*- f90 -*-
>> +! Platform-specific declarations of SIMD math functions for Fortran.
>
> Let's keep the -*- f90 -*- annotations, they will also be helpful for
> future people editing these files.  For instance, on my computer, both
> emacs and vim use the Fortran syntax highlighting rules for these
> files when the annotation is present, and the C rules when it's
> absent.
>
>> +            # Skip Fortran header files
>> +            if 'finclude' in header:
>> +                continue
>
> I think this conditional should be
>
>     if header.startswith('finclude/') or '/finclude/' in header
>
> so that it only applies to headers in the directory 'finclude', not
> any hypothetical future headers that happen to have the string
> 'finclude' in their base names.  (I don't know why that would happen,
> but if it ever did, I can see someone being really confused why it
> wasn't getting tested, so some extra defensiveness now seems like a
> good idea.)

I agree.  Maybe also add a period to the end of the comment. 8-)

Thanks,
Florian
Martin Liška March 1, 2019, 9:31 a.m. UTC | #14
On 2/28/19 2:32 PM, Florian Weimer wrote:
> * Zack Weinberg:
> 
>> Revised patch looks good to me except:
>>
>>> -! Platform-specific declarations of SIMD math functions for Fortran. -*- f90 -*-
>>> +! Platform-specific declarations of SIMD math functions for Fortran.
>>
>> Let's keep the -*- f90 -*- annotations, they will also be helpful for
>> future people editing these files.  For instance, on my computer, both
>> emacs and vim use the Fortran syntax highlighting rules for these
>> files when the annotation is present, and the C rules when it's
>> absent.
>>
>>> +            # Skip Fortran header files
>>> +            if 'finclude' in header:
>>> +                continue
>>
>> I think this conditional should be
>>
>>     if header.startswith('finclude/') or '/finclude/' in header
>>
>> so that it only applies to headers in the directory 'finclude', not
>> any hypothetical future headers that happen to have the string
>> 'finclude' in their base names.  (I don't know why that would happen,
>> but if it ever did, I can see someone being really confused why it
>> wasn't getting tested, so some extra defensiveness now seems like a
>> good idea.)
> 
> I agree.  Maybe also add a period to the end of the comment. 8-)
> 
> Thanks,
> Florian
> 

Thank you both for help, I've updated the patch.

Martin
From 74b9d0deddbcb0f6433be937f6b1d7f632e257d6 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 26 Feb 2019 18:17:36 +0100
Subject: [PATCH] Fix location where math-vector-fortran.h is installed.

ChangeLog:

2019-02-26  Martin Liska  <mliska@suse.cz>

	* math/Makefile: Change location where math-vector-fortran.h is
	installed.
	* math/finclude/math-vector-fortran.h: Move from bits/math-vector-fortran.h.
	* sysdeps/x86/fpu/finclude/math-vector-fortran.h: Move
	from sysdeps/x86/fpu/bits/math-vector-fortran.h.
	* scripts/check-installed-headers.sh: Skip Fortran header files.
	* scripts/check-wrapper-headers.py: Likewise.
---
 math/Makefile                                            | 2 +-
 {bits => math/finclude}/math-vector-fortran.h            | 0
 scripts/check-installed-headers.sh                       | 4 ++++
 scripts/check-wrapper-headers.py                         | 4 ++++
 sysdeps/x86/fpu/{bits => finclude}/math-vector-fortran.h | 0
 5 files changed, 9 insertions(+), 1 deletion(-)
 rename {bits => math/finclude}/math-vector-fortran.h (100%)
 rename sysdeps/x86/fpu/{bits => finclude}/math-vector-fortran.h (100%)

diff --git a/math/Makefile b/math/Makefile
index fc4191089d..cb4eaec6a9 100644
--- a/math/Makefile
+++ b/math/Makefile
@@ -26,7 +26,7 @@ headers		:= math.h bits/mathcalls.h bits/mathinline.h \
 		   fpu_control.h complex.h bits/cmathcalls.h fenv.h \
 		   bits/fenv.h bits/fenvinline.h bits/mathdef.h tgmath.h \
 		   bits/math-finite.h bits/math-vector.h \
-		   bits/math-vector-fortran.h \
+		   finclude/math-vector-fortran.h \
 		   bits/libm-simd-decl-stubs.h bits/iscanonical.h \
 		   bits/flt-eval-method.h bits/fp-fast.h bits/fp-logb.h \
 		   bits/long-double.h bits/mathcalls-helper-functions.h \
diff --git a/bits/math-vector-fortran.h b/math/finclude/math-vector-fortran.h
similarity index 100%
rename from bits/math-vector-fortran.h
rename to math/finclude/math-vector-fortran.h
diff --git a/scripts/check-installed-headers.sh b/scripts/check-installed-headers.sh
index 8e7beffd82..1f4496446c 100644
--- a/scripts/check-installed-headers.sh
+++ b/scripts/check-installed-headers.sh
@@ -84,6 +84,10 @@ for header in "$@"; do
         (sys/elf.h)
             continue;;
 
+        # Skip Fortran headers.
+        (finclude/*)
+            continue;;
+
 	# sys/sysctl.h is unsupported for x32.
 	(sys/sysctl.h)
             case "$is_x32" in
diff --git a/scripts/check-wrapper-headers.py b/scripts/check-wrapper-headers.py
index 094faa3ced..dc9fd86063 100644
--- a/scripts/check-wrapper-headers.py
+++ b/scripts/check-wrapper-headers.py
@@ -75,6 +75,10 @@ def check_headers(args):
 
         is_nonsysdep_header = os.access(header, os.R_OK)
         if is_nonsysdep_header:
+            # Skip Fortran header files.
+            if '/finclude/' in header:
+                continue
+
             include_path = os.path.join(args.root, INCLUDE, header)
             if not os.access(include_path, os.R_OK):
                 print('error: missing wrapper header {} for {}'.format(
diff --git a/sysdeps/x86/fpu/bits/math-vector-fortran.h b/sysdeps/x86/fpu/finclude/math-vector-fortran.h
similarity index 100%
rename from sysdeps/x86/fpu/bits/math-vector-fortran.h
rename to sysdeps/x86/fpu/finclude/math-vector-fortran.h
Steve Ellcey March 1, 2019, 10:30 p.m. UTC | #15
On Fri, 2019-03-01 at 10:31 +0100, Martin Liška wrote:
> 
> Thank you both for help, I've updated the patch.
> 
> Martin

Martin,

I tested this patch with some vector routines I am experimenting with
on Aarch64 and it worked fine for me when I created a
sysdeps/aarch64/fpu/finclude/math-vector-fortran.h file.

Steve Ellcey
sellcey@marvell.com
Martin Liška March 5, 2019, 12:03 p.m. UTC | #16
On 3/1/19 10:31 AM, Martin Liška wrote:
> Thank you both for help, I've updated the patch.

Do it look fine to be installed?

Thanks,
Martin
Joseph Myers March 6, 2019, 9:13 p.m. UTC | #17
On Tue, 5 Mar 2019, Martin Liška wrote:

> On 3/1/19 10:31 AM, Martin Liška wrote:
> > Thank you both for help, I've updated the patch.
> 
> Do it look fine to be installed?

Yes.
diff mbox series

Patch

From ccc30d7ac8ae5383e3e876bbd0da88640aefd4a0 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 26 Feb 2019 18:17:36 +0100
Subject: [PATCH] Fix location where math-vector-fortran.h is installed.

ChangeLog:

2019-02-26  Martin Liska  <mliska@suse.cz>

	* math/Makefile: Change location where math-vector-fortran.h is
	installed.
	* finclude/math-vector-fortran.h: Move from bits/math-vector-fortran.h.
	* sysdeps/x86/fpu/finclude/math-vector-fortran.h: Move
	from sysdeps/x86/fpu/bits/math-vector-fortran.h.
---
 {bits => finclude}/math-vector-fortran.h                 | 0
 math/Makefile                                            | 2 +-
 sysdeps/x86/fpu/{bits => finclude}/math-vector-fortran.h | 0
 3 files changed, 1 insertion(+), 1 deletion(-)
 rename {bits => finclude}/math-vector-fortran.h (100%)
 rename sysdeps/x86/fpu/{bits => finclude}/math-vector-fortran.h (100%)

diff --git a/bits/math-vector-fortran.h b/finclude/math-vector-fortran.h
similarity index 100%
rename from bits/math-vector-fortran.h
rename to finclude/math-vector-fortran.h
diff --git a/math/Makefile b/math/Makefile
index fc4191089d..cb4eaec6a9 100644
--- a/math/Makefile
+++ b/math/Makefile
@@ -26,7 +26,7 @@  headers		:= math.h bits/mathcalls.h bits/mathinline.h \
 		   fpu_control.h complex.h bits/cmathcalls.h fenv.h \
 		   bits/fenv.h bits/fenvinline.h bits/mathdef.h tgmath.h \
 		   bits/math-finite.h bits/math-vector.h \
-		   bits/math-vector-fortran.h \
+		   finclude/math-vector-fortran.h \
 		   bits/libm-simd-decl-stubs.h bits/iscanonical.h \
 		   bits/flt-eval-method.h bits/fp-fast.h bits/fp-logb.h \
 		   bits/long-double.h bits/mathcalls-helper-functions.h \
diff --git a/sysdeps/x86/fpu/bits/math-vector-fortran.h b/sysdeps/x86/fpu/finclude/math-vector-fortran.h
similarity index 100%
rename from sysdeps/x86/fpu/bits/math-vector-fortran.h
rename to sysdeps/x86/fpu/finclude/math-vector-fortran.h
-- 
2.20.1