diff mbox

Merge from gomp-4_1-branch to trunk

Message ID 561E0598.6030003@embedded-brains.de
State New
Headers show

Commit Message

Sebastian Huber Oct. 14, 2015, 7:34 a.m. UTC
Hello,

I get now the following error:

libtool: compile: /scratch/git-build/b-gcc-git-arm-rtems4.12/./gcc/xgcc 
-B/scratch/git-build/b-gcc-git-arm-rtems4.12/./gcc/ -nostdinc 
-B/scratch/git-build/b-gcc-git-arm-rtems4.12/arm-rtems4.12/newlib/ 
-isystem 
/scratch/git-build/b-gcc-git-arm-rtems4.12/arm-rtems4.12/newlib/targ-include 
-isystem /home/EB/sebastian_h/archive/gcc-git/newlib/libc/include 
-B/opt/rtems-4.12/arm-rtems4.12/bin/ 
-B/opt/rtems-4.12/arm-rtems4.12/lib/ -isystem 
/opt/rtems-4.12/arm-rtems4.12/include -isystem 
/opt/rtems-4.12/arm-rtems4.12/sys-include -DHAVE_CONFIG_H -I. 
-I/home/EB/sebastian_h/archive/gcc-git/libgomp 
-I/home/EB/sebastian_h/archive/gcc-git/libgomp/config/rtems 
-I/home/EB/sebastian_h/archive/gcc-git/libgomp/config/posix 
-I/home/EB/sebastian_h/archive/gcc-git/libgomp 
-I/home/EB/sebastian_h/archive/gcc-git/libgomp/../include -Wall -Werror 
-g -O2 -MT fortran.lo -MD -MP -MF .deps/fortran.Tpo -c 
/home/EB/sebastian_h/archive/gcc-git/libgomp/fortran.c -o fortran.o
/home/EB/sebastian_h/archive/gcc-git/libgomp/fortran.c: In function 
'omp_get_place_proc_ids_':
/home/EB/sebastian_h/archive/gcc-git/libgomp/fortran.c:484:39: error: 
passing argument 2 of 'omp_get_place_proc_ids' from incompatible pointer 
type [-Werror=incompatible-pointer-types]
    omp_get_place_proc_ids (*place_num, ids);
                                        ^
In file included from 
/home/EB/sebastian_h/archive/gcc-git/libgomp/fortran.c:28:0:
/home/EB/sebastian_h/archive/gcc-git/libgomp/fortran.c:73:18: note: 
expected 'int *' but argument is of type 'int32_t * {aka long int *}'
  ialias_redirect (omp_get_place_proc_ids)
                   ^
/home/EB/sebastian_h/archive/gcc-git/libgomp/libgomp.h:1011:24: note: in 
definition of macro 'ialias_redirect'
    extern __typeof (fn) fn __asm__ (ialias_ulp "gomp_ialias_" #fn) 
attribute_hidden;
                         ^
/home/EB/sebastian_h/archive/gcc-git/libgomp/fortran.c: In function 
'omp_get_partition_place_nums_':
/home/EB/sebastian_h/archive/gcc-git/libgomp/fortran.c:508:33: error: 
passing argument 1 of 'omp_get_partition_place_nums' from incompatible 
pointer type [-Werror=incompatible-pointer-types]
    omp_get_partition_place_nums (place_nums);
                                  ^
In file included from 
/home/EB/sebastian_h/archive/gcc-git/libgomp/fortran.c:28:0:
/home/EB/sebastian_h/archive/gcc-git/libgomp/fortran.c:76:18: note: 
expected 'int *' but argument is of type 'int32_t * {aka long int *}'
  ialias_redirect (omp_get_partition_place_nums)
                   ^
/home/EB/sebastian_h/archive/gcc-git/libgomp/libgomp.h:1011:24: note: in 
definition of macro 'ialias_redirect'
    extern __typeof (fn) fn __asm__ (ialias_ulp "gomp_ialias_" #fn) 
attribute_hidden;

We have for example (libgomp/omp_lib.f90.in):

           subroutine omp_get_place_proc_ids (place_num, ids)
             integer (4), intent(in) :: place_num
             integer (4), intent(out) :: ids(*)
           end subroutine omp_get_place_proc_ids

So this interface is different to (libgomp/omp.h.in):

extern void omp_get_place_proc_ids (int, int *) __GOMP_NOTHROW;

The following patch fixes the problem, but I am not sure if this is 
really the best way to address this issue:

Comments

Jakub Jelinek Oct. 14, 2015, 8:04 a.m. UTC | #1
On Wed, Oct 14, 2015 at 09:34:48AM +0200, Sebastian Huber wrote:
> /home/EB/sebastian_h/archive/gcc-git/libgomp/fortran.c:28:0:
> /home/EB/sebastian_h/archive/gcc-git/libgomp/fortran.c:73:18: note: expected
> 'int *' but argument is of type 'int32_t * {aka long int *}'

Ugh, wasn't aware that some targets use long int for int32_t :(.

> The following patch fixes the problem, but I am not sure if this is really
> the best way to address this issue:
> 
> diff --git a/libgomp/fortran.c b/libgomp/fortran.c
> index ceff9ac..44aaf92 100644
> --- a/libgomp/fortran.c
> +++ b/libgomp/fortran.c
> @@ -481,7 +481,9 @@ omp_get_place_num_procs_8_ (const int64_t *place_num)
>  void
>  omp_get_place_proc_ids_ (const int32_t *place_num, int32_t *ids)
>  {
> -  omp_get_place_proc_ids (*place_num, ids);
> +  int int_ids;
> +  omp_get_place_proc_ids (*place_num, &int_ids);
> +  *ids = int_ids;
>  }
> 
>  void
> @@ -505,7 +507,9 @@ omp_get_partition_num_places_ (void)
>  void
>  omp_get_partition_place_nums_ (int32_t *place_nums)
>  {
> -  omp_get_partition_place_nums (place_nums);
> +  int int_place_nums;
> +  omp_get_partition_place_nums (&int_place_nums);
> +  *place_nums = int_place_nums;
>  }

No, both the above changes are wrong.  There is not a single int32_t
written, but could be many more, it is an array of 32-bit integers.
I'd say you just want to cast explicitly,
  omp_get_place_proc_ids (*place_num, (int *) ids);
and
  omp_get_parition_place_nums ((int *) place_nums);
The reason for int32_t is that on the Fortran side it is integer(kind=4)
and everywhere else for that int32_t is used.
If this works, the patch is preapproved.

As for aliasing, it will always be int stores vs. integer(kind=4) reads,
int32_t is just a type used in the wrappers.

	Jakub
Sebastian Huber Oct. 14, 2015, 8:10 a.m. UTC | #2
On 14/10/15 10:04, Jakub Jelinek wrote:
> On Wed, Oct 14, 2015 at 09:34:48AM +0200, Sebastian Huber wrote:
>> >/home/EB/sebastian_h/archive/gcc-git/libgomp/fortran.c:28:0:
>> >/home/EB/sebastian_h/archive/gcc-git/libgomp/fortran.c:73:18: note: expected
>> >'int *' but argument is of type 'int32_t * {aka long int *}'
> Ugh, wasn't aware that some targets use long int for int32_t:(.
>

This is actually a feature of the newlib-stdint.h:

[...]
/* newlib uses 32-bit long in certain cases for all non-SPU
    targets.  */
#ifndef STDINT_LONG32
#define STDINT_LONG32 (LONG_TYPE_SIZE == 32)
#endif

#define SIG_ATOMIC_TYPE "int"

/* The newlib logic actually checks for sizes greater than 32 rather
    than equal to 64 for various 64-bit types.  */

#define INT8_TYPE (CHAR_TYPE_SIZE == 8 ? "signed char" : 0)
#define INT16_TYPE (SHORT_TYPE_SIZE == 16 ? "short int" : INT_TYPE_SIZE 
== 16 ? "int" : CHAR_TYPE_SIZE == 16 ? "signed char" : 0)
#define INT32_TYPE (STDINT_LONG32 ? "long int" : INT_TYPE_SIZE == 32 ? 
"int" : SHORT_TYPE_SIZE == 32 ? "short int" : CHAR_TYPE_SIZE == 32 ? 
"signed char" : 0)
[...]

This regularly causes problems like this. In addition it leads to a 
complicated PRI* macro definition in <inttypes.h>.
Sebastian Huber Oct. 14, 2015, 8:32 a.m. UTC | #3
On 14/10/15 10:04, Jakub Jelinek wrote:
> No, both the above changes are wrong.  There is not a single int32_t
> written, but could be many more, it is an array of 32-bit integers.
> I'd say you just want to cast explicitly,
>    omp_get_place_proc_ids (*place_num, (int *) ids);
> and
>    omp_get_parition_place_nums ((int *) place_nums);
> The reason for int32_t is that on the Fortran side it is integer(kind=4)
> and everywhere else for that int32_t is used.
> If this works, the patch is preapproved.

I checked this in:

https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=228805
diff mbox

Patch

diff --git a/libgomp/fortran.c b/libgomp/fortran.c
index ceff9ac..44aaf92 100644
--- a/libgomp/fortran.c
+++ b/libgomp/fortran.c
@@ -481,7 +481,9 @@  omp_get_place_num_procs_8_ (const int64_t *place_num)
  void
  omp_get_place_proc_ids_ (const int32_t *place_num, int32_t *ids)
  {
-  omp_get_place_proc_ids (*place_num, ids);
+  int int_ids;
+  omp_get_place_proc_ids (*place_num, &int_ids);
+  *ids = int_ids;
  }

  void
@@ -505,7 +507,9 @@  omp_get_partition_num_places_ (void)
  void
  omp_get_partition_place_nums_ (int32_t *place_nums)
  {
-  omp_get_partition_place_nums (place_nums);
+  int int_place_nums;
+  omp_get_partition_place_nums (&int_place_nums);
+  *place_nums = int_place_nums;
  }

  void