Re: OpenACC dimension range propagation optimization

[Richard Biener <richard dot guenther at gmail dot com>]

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?  Is the call argument always a constant
that fits in a HOST_WIDE_INT (or even int here)?  Are there always enough
'dims' in the tree list?  Is the 'dim' value always an INTEGER_CST that
fits a HOST_WIDE_INT (or even an int here)?

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

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

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);

so you get at least [1, +INF] and [0, +INF] here.

Thanks,
Richard.

> nathan