[Patch, omp] Patch to omp-low.c to fix failures on IA64 HP-UX

[Patch, omp] Patch to omp-low.c to fix failures on IA64 HP-UX

[Steve Ellcey]

I have two libgomp tests, libgomp.c/loop-5.c and libgomp.c++/loop-8.C
that fail on IA64 HP-UX in 32 bit mode.  In this mode pointers are 32
bits but need to be extended to 64 bits before being dereferenced.  IA64
has a  special instruction, addp4 to do that.  In omp-low.c
(expand_omp_for_generic) we have these two instructions:

	t1 = fold_convert (fd->iter_type, fd->loop.n2);
	t0 = fold_convert (fd->iter_type, fd->loop.n1);

Where fd->loop.n1 (and n2) are 32 bit pointer types and fd->iter_type is
a 64 bit 'unsigned long long' type.  There are two ways of getting from
a 32 bit pointer to a 64 bit unsigned long, one could use addp4 or one
could convert the pointer to a 32 bit unsigned long and then zero extend
that.

It turns out that using either method works as long as you use the same
method for both of these statements but in the two failing tests one
convert gets done one way and the other the other way and then the test
fails.  They get done differently because one of the expressions being
converted is a simple address and the other is an expression and this
results in different choices in how to extend the expression.

This patch fixes the problem by explicitly converting the pointer to
'unsigned long' first and then extending that to type 'unsigned long
long'.  This forces a consistent conversion path for both expressions
and fixes the bug.

Tested on IA64 HP-UX and Linux with no regressions.

OK for checkin?


2008-10-30  Steve Ellcey  <sje@cup.hp.com>

	* omp-low.c (expand_omp_for_generic): Change conversion code.


Index: omp-low.c
===================================================================
--- omp-low.c	(revision 141457)
+++ omp-low.c	(working copy)
@@ -3681,8 +3681,20 @@ expand_omp_for_generic (struct omp_regio
       t4 = build_fold_addr_expr (iend0);
       t3 = build_fold_addr_expr (istart0);
       t2 = fold_convert (fd->iter_type, fd->loop.step);
-      t1 = fold_convert (fd->iter_type, fd->loop.n2);
-      t0 = fold_convert (fd->iter_type, fd->loop.n1);
+      if (POINTER_TYPE_P (TREE_TYPE (fd->loop.n2))
+	  && (TYPE_PRECISION (fd->iter_type) >
+	      TYPE_PRECISION (long_unsigned_type_node)))
+	t1 = fold_convert (fd->iter_type,
+			   fold_convert (long_unsigned_type_node, fd->loop.n2));
+      else
+	t1 = fold_convert (fd->iter_type, fd->loop.n2);
+      if (POINTER_TYPE_P (TREE_TYPE (fd->loop.n1))
+	  && (TYPE_PRECISION (fd->iter_type) >
+	      TYPE_PRECISION (long_unsigned_type_node)))
+	t0 = fold_convert (fd->iter_type,
+			   fold_convert (long_unsigned_type_node, fd->loop.n1));
+      else
+        t0 = fold_convert (fd->iter_type, fd->loop.n1);
       if (bias)
 	{
 	  t1 = fold_build2 (PLUS_EXPR, fd->iter_type, t1, bias);

Re: [Patch, omp] Patch to omp-low.c to fix failures on IA64 HP-UX

[Richard Guenther]

On Thu, Oct 30, 2008 at 11:02 PM, Steve Ellcey <sje@cup.hp.com> wrote:
> I have two libgomp tests, libgomp.c/loop-5.c and libgomp.c++/loop-8.C
> that fail on IA64 HP-UX in 32 bit mode.  In this mode pointers are 32
> bits but need to be extended to 64 bits before being dereferenced.  IA64
> has a  special instruction, addp4 to do that.  In omp-low.c
> (expand_omp_for_generic) we have these two instructions:
>
>        t1 = fold_convert (fd->iter_type, fd->loop.n2);
>        t0 = fold_convert (fd->iter_type, fd->loop.n1);
>
> Where fd->loop.n1 (and n2) are 32 bit pointer types and fd->iter_type is
> a 64 bit 'unsigned long long' type.  There are two ways of getting from
> a 32 bit pointer to a 64 bit unsigned long, one could use addp4 or one
> could convert the pointer to a 32 bit unsigned long and then zero extend
> that.

It turns out that the middle-end type checker would error on a conversion
from a pointer to an integer type of different precision.  Just it is never
executed after gimplification ... (and we specifically allow conversion
to/from sizetype to make ports with mismatching pointer/sizetype precision
happy).

For 4.5 I plan to enable this checking code after all passes which will
catch this errorneous situation.

> It turns out that using either method works as long as you use the same
> method for both of these statements but in the two failing tests one
> convert gets done one way and the other the other way and then the test
> fails.  They get done differently because one of the expressions being
> converted is a simple address and the other is an expression and this
> results in different choices in how to extend the expression.
>
> This patch fixes the problem by explicitly converting the pointer to
> 'unsigned long' first and then extending that to type 'unsigned long
> long'.  This forces a consistent conversion path for both expressions
> and fixes the bug.
>
> Tested on IA64 HP-UX and Linux with no regressions.
>
> OK for checkin?

I'll leave that to OMP maintainers.  It at least looks ugly ;)

Richard.

>
> 2008-10-30  Steve Ellcey  <sje@cup.hp.com>
>
>        * omp-low.c (expand_omp_for_generic): Change conversion code.
>
>
> Index: omp-low.c
> ===================================================================
> --- omp-low.c   (revision 141457)
> +++ omp-low.c   (working copy)
> @@ -3681,8 +3681,20 @@ expand_omp_for_generic (struct omp_regio
>       t4 = build_fold_addr_expr (iend0);
>       t3 = build_fold_addr_expr (istart0);
>       t2 = fold_convert (fd->iter_type, fd->loop.step);
> -      t1 = fold_convert (fd->iter_type, fd->loop.n2);
> -      t0 = fold_convert (fd->iter_type, fd->loop.n1);
> +      if (POINTER_TYPE_P (TREE_TYPE (fd->loop.n2))
> +         && (TYPE_PRECISION (fd->iter_type) >
> +             TYPE_PRECISION (long_unsigned_type_node)))
> +       t1 = fold_convert (fd->iter_type,
> +                          fold_convert (long_unsigned_type_node, fd->loop.n2));
> +      else
> +       t1 = fold_convert (fd->iter_type, fd->loop.n2);
> +      if (POINTER_TYPE_P (TREE_TYPE (fd->loop.n1))
> +         && (TYPE_PRECISION (fd->iter_type) >
> +             TYPE_PRECISION (long_unsigned_type_node)))
> +       t0 = fold_convert (fd->iter_type,
> +                          fold_convert (long_unsigned_type_node, fd->loop.n1));
> +      else
> +        t0 = fold_convert (fd->iter_type, fd->loop.n1);
>       if (bias)
>        {
>          t1 = fold_build2 (PLUS_EXPR, fd->iter_type, t1, bias);
>

Re: [Patch, omp] Patch to omp-low.c to fix failures on IA64 HP-UX

[Jakub Jelinek]

On Thu, Oct 30, 2008 at 03:02:38PM -0700, Steve Ellcey wrote:
> Tested on IA64 HP-UX and Linux with no regressions.

Using long_unsigned_type_node in this case is wrong, that may be correct for
IA-64 32-bit HPUX, but not generally.  The reason why iter_type in omp-low.c
is either long_integer_type_node, or long_long_unsigned_type_node is that
those are the types of the iterators in libgomp (for signed and unsigned
loops).  But in this case you want to cast to an integer with the same
precision as the pointer.

So IMHO you want something like this instead.  type_for_size langhook
might not be best for LTO SSA expansion, but it is ATM used all around
omp expansion, so we can use it here too for now and when LTO needs it, switch to
build_nonstandard_integer_type or something better (ideally something that
caches the types rather than creating a new one each time).
Can you test it on HPUX?

--- gcc/omp-low.c.jj	2008-09-30 16:57:11.000000000 +0200
+++ gcc/omp-low.c	2008-10-31 13:03:00.000000000 +0100
@@ -3681,8 +3681,20 @@ expand_omp_for_generic (struct omp_regio
       t4 = build_fold_addr_expr (iend0);
       t3 = build_fold_addr_expr (istart0);
       t2 = fold_convert (fd->iter_type, fd->loop.step);
-      t1 = fold_convert (fd->iter_type, fd->loop.n2);
-      t0 = fold_convert (fd->iter_type, fd->loop.n1);
+      if (POINTER_TYPE_P (type)
+	  && TYPE_PRECISION (type) != TYPE_PRECISION (fd->iter_type))
+	{
+	  /* Avoid casting pointers to integer of a different size.  */
+	  tree itype
+	    = lang_hooks.types.type_for_size (TYPE_PRECISION (itype), 0);
+	  t1 = fold_convert (fd->iter_type, fold_convert (itype, fd->loop.n2));
+	  t0 = fold_convert (fd->iter_type, fold_convert (itype, fd->loop.n1));
+	}
+      else
+	{
+	  t1 = fold_convert (fd->iter_type, fd->loop.n2);
+	  t0 = fold_convert (fd->iter_type, fd->loop.n1);
+	}
       if (bias)
 	{
 	  t1 = fold_build2 (PLUS_EXPR, fd->iter_type, t1, bias);

	Jakub

Re: [Patch, omp] Patch to omp-low.c to fix failures on IA64 HP-UX

[Steve Ellcey]

On Fri, 2008-10-31 at 13:16 +0100, Jakub Jelinek wrote:

> Can you test it on HPUX?
> 
> --- gcc/omp-low.c.jj	2008-09-30 16:57:11.000000000 +0200
> +++ gcc/omp-low.c	2008-10-31 13:03:00.000000000 +0100
> @@ -3681,8 +3681,20 @@ expand_omp_for_generic (struct omp_regio
>        t4 = build_fold_addr_expr (iend0);
>        t3 = build_fold_addr_expr (istart0);
>        t2 = fold_convert (fd->iter_type, fd->loop.step);
> -      t1 = fold_convert (fd->iter_type, fd->loop.n2);
> -      t0 = fold_convert (fd->iter_type, fd->loop.n1);
> +      if (POINTER_TYPE_P (type)
> +	  && TYPE_PRECISION (type) != TYPE_PRECISION (fd->iter_type))
> +	{
> +	  /* Avoid casting pointers to integer of a different size.  */
> +	  tree itype
> +	    = lang_hooks.types.type_for_size (TYPE_PRECISION (itype), 0);

I am assuming that should be "TYPE_PRECISION (type)", not
"TYPE_PRECISION (itype)" in the type_for_size call.  I will test the
patch with that change.

Steve Ellcey
sje@cup.hp.com