diff mbox

Default compute dimensions

Message ID 20160201184237.GI3017@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Feb. 1, 2016, 6:42 p.m. UTC 
On Mon, Feb 01, 2016 at 11:15:13AM -0500, Nathan Sidwell wrote:
> On 02/01/16 10:32, Jakub Jelinek wrote:
> >On Mon, Feb 01, 2016 at 09:15:05AM -0500, Nathan Sidwell wrote:
> >>On 01/29/16 10:18, Jakub Jelinek wrote:
> >>>On Thu, Jan 28, 2016 at 10:38:51AM -0500, Nathan Sidwell wrote:
> >>>>This patch adds default compute dimension handling.  Users rarely specify
> >>>>compute dimensions, expecting the toolchain to DTRT.  More savvy users would
> >>>>like to specify global defaults.  This patch permits both.
> >>>
> >>>Isn't it better to be able to override the defaults on the library side?
> >>>I mean, when when somebody is compiling the code, often he doesn't know the
> >>>exact properties of the hw it will be run on, if he does, I think it is
> >>>better to specify them explicitly in the code.
> >>
> >>I realized that it's actually not possible to markup the code in this way,
> >>as an 'intermediate' user.  One can exercise complete control by saying
> >>exactly the axis/axes over which a loop is to be partitioned, and then
> >>specify the geometry.  But one cannot use the 'auto' feature and have the
> >>compiler choose an axis without also relying on the compiler choosing a size
> >>for that axis.  As I already said,  IMHO being able to specify a
> >>compile-time size is useful.
> >
> >Ok, I won't fight against it.  But please make sure it can be overridden on
> >the library side too.
> 
> Absolutely, thanks!

Your patch broke bootstrap on ILP32 hosts, I'm testing following fix.
Supporting unsigned values from 0x80000000U to 0xffffffffU only on LP64
hosts and not on ILP64 hosts sounds really weird, I think it is better
to only support 1 to 0x7fffffffU.

2016-02-01  Jakub Jelinek  <jakub@redhat.com>

	* omp-low.c (oacc_parse_default_dims): Avoid
	-Wsign-compare warning, make sure value fits into int
	rather than just unsigned int.



	Jakub

Comments

Nathan Sidwell Feb. 1, 2016, 7 p.m. UTC | #1 
On 02/01/16 13:42, Jakub Jelinek wrote:

> Your patch broke bootstrap on ILP32 hosts, I'm testing following fix.
> Supporting unsigned values from 0x80000000U to 0xffffffffU only on LP64
> hosts and not on ILP64 hosts sounds really weird, I think it is better
> to only support 1 to 0x7fffffffU.

yes, I must have missed that first cast when changing my mind over 
signed/unsigned.  thanks!

nathan
diff mbox

Patch

--- gcc/omp-low.c.jj	2016-02-01 19:08:51.000000000 +0100
+++ gcc/omp-low.c	2016-02-01 19:36:57.751641369 +0100
@@ -20285,10 +20285,10 @@  oacc_parse_default_dims (const char *dim
 
 	      errno = 0;
 	      val = strtol (pos, CONST_CAST (char **, &eptr), 10);
-	      if (errno || val <= 0 || (unsigned)val != val)
+	      if (errno || val <= 0 || (int) val != val)
 		goto malformed;
 	      pos = eptr;
-	      oacc_default_dims[ix] = (int)val;
+	      oacc_default_dims[ix] = (int) val;
 	    }
 	}
       if (*pos)