Re: OpenACC dimension range propagation optimization

[Nathan Sidwell <nathan at acm dot org>]

On 11/04/15 05:26, Richard Biener wrote:
> On Tue, Nov 3, 2015 at 7:11 PM, Nathan Sidwell <nathan@acm.org> wrote:
> > Richard,
> > this patch implements VRP for the 2 openacc axis internal fns I've added.
> > We know the position within  a dimension cannot exceed that dimensions
> > extend. Further, if the extend is dynamic, the target backend may well know
> > there's a hardware-mandated maximum value.
> >
> > Hence, added a new target hook to allow the backend to specify that upper
> > bound, and added smarts to extract_range_basic to process the two internal
> > functions.
> >
> > Incidentally, this was the bit I was working on at the cauldron, which
> > caused me to work on the min/max range combining.
> >
> > ok for trunk?
>
> +         /* Optimizing these two internal functions helps the loop
> +            optimizer eliminate outer comparisons.  Size is [1,N]
> +            and pos is [0,N-1].  */
> +         {
> +           bool is_pos = ifn_code == IFN_GOACC_DIM_POS;
> +           tree attr = get_oacc_fn_attrib (current_function_decl);
> +           tree arg = gimple_call_arg (stmt, 0);
> +           int axis = TREE_INT_CST_LOW (arg);
> +           tree dims = TREE_VALUE (attr);
> +
> +           for (int ix = axis; ix--;)
> +             dims = TREE_CHAIN (dims);
> +           int size = TREE_INT_CST_LOW (TREE_VALUE (dims));
> +
> +           if (!size)
> +             /* If it's dynamic, the backend might know a hardware
> +                limitation.  */
> +             size = targetm.goacc.dim_limit (axis);
>
> this all seems a little bit fragile and relying on implementation details?
> Is the attribute always present?

Yes.  The ifns are inserted by a pass (execute_oacc_device_lower) that only 
operates on such functions. (I considered adding  an assert, but figured the 
resulting segfault  would be loud enough)

> Is the call argument always a constant
> that fits in a HOST_WIDE_INT (or even int here)?

Yes.


Are there always enough
> 'dims' in the tree list?

Yes.  The oacc_device_lower pass has already validated and canonicalized the 
dimensions.


Is the 'dim' value always an INTEGER_CST that
> fits a HOST_WIDE_INT (or even an int here)?

Yes.  That's part of the validation.  see set_oacc_fn_attrib (omp-low.c)

> I hope all these constraints (esp. 'int' fitting) are verified by the parser.

It's an internal function not visible the user.  Generation is entirely within 
the compiler

> If so I'd like to see helper functions to hide these implementation details
> from generic code like this.

ok.

> You miss to provide the known lower bound to VRP when size is 0
> in the end.  Just unconditioonally do
>
>    tree type = TREE_TYPE (gimple_call_lhs (stmt));
>    set_value_range (vr, VR_RANGE,
>                             build_int_cst (type, is_pos ? 0 : 1),
>                             size
>                             ? build_int_cst (type, size - is_pos)
>                             : vrp_val_max (type), NULL);

I'm confused.  If size is zero, we never execute that path, and IIUC therefore 
never specify a range.  What you suggest looks like an additional improvement 
though.  Is that what you meant?

nathan