Re: [Patch] OpenMP/Fortran: Partially fix non-rect loop nests [PR107424]

[Jakub Jelinek <jakub@redhat.com>]

On Thu, Jan 19, 2023 at 03:40:19PM +0100, Tobias Burnus wrote:
> +  gfc_symbol *var = code->ext.iterator->var->symtree->n.sym;
> +
> +  gfc_se se;
> +  tree tree_var, a1, a2;
> +  a1 = integer_one_node;
> +  a2 = integer_zero_node;
> +
> +  gfc_init_se (&se, NULL);
> +  gfc_conv_expr_lhs (&se, code->ext.iterator->var);
> +  gfc_add_block_to_block (pblock, &se.pre);
> +  tree_var = se.expr;
> +
> +  {
> +    /* FIXME: Handle non-unity iterations, cf. PR fortran/107424.

I think instead of non-unity etc. it is better to talk about
constant step 1 or -1.

> +       The issue is that for those a 'count' variable is used.  */
> +    dovar_init *di;
> +    unsigned ix;
> +    tree t = tree_var;
> +    while (TREE_CODE (t) == INDIRECT_REF)
> +      t = TREE_OPERAND (t, 0);
> +    FOR_EACH_VEC_ELT (*inits, ix, di)
> +      {
> +	tree t2 = di->var;
> +	while (TREE_CODE (t2) == INDIRECT_REF)
> +	  t2 = TREE_OPERAND (t2, 0);

The actual problem with non-simple loops for non-rectangular loops is
both in case it is an inner loop which uses some outer loop's iterator,
or if it is outer loop whose iterator is used, both of those cases
will not be handled properly.  The former case because instead of
having lb and ub expressions in canonicalized form var-outer * m + a
lb will be 0 (that is fine) and ub will be
(var-outer * m2 + a2 + step - var-outer * m1 - a1) / step
or so (sure, we can simplify that to
(var-outer * (m1 - m2) + (a2 + step - a1)) / step
but the division remains.  And the latter case is bad because we
need var-outer but we actually compute some artificial count iterator
and var-outer is only initialized in the body of the loop.
These sorry_at seems to handle just one of those, when the outer
loop whose var-outer is referenced is not simple, no?

I wonder if it wouldn't be cleaner and easier to simply remember for
each loop in XALLOCAVEC array whether it was simple or not and why
(from the:
      if (VAR_P (dovar))
        {
          if (integer_onep (step))
            simple = 1;
          else if (tree_int_cst_equal (step, integer_minus_one_node))
            simple = -1;
        }
      else
        dovar_decl
          = gfc_trans_omp_variable (code->ext.iterator->var->symtree->n.sym,
                                    false);
remember if it was simple (1/-1) or VAR_P !simple (then we would
if needed for non-rect sorry_at about step not being constant 1 or -1)
or if it is the !VAR_P case.
And then the non-rect sorry can be emitted for both the cases easily
(especially if you precompute the:
      if (VAR_P (dovar))
        {
          if (integer_onep (step))
            simple_loop[i] = 1;
          else if (tree_int_cst_equal (step, integer_minus_one_node))
            simple_loop[i] = -1;
	  else
	    simple_loop[i] = 0;
        }
      else
	simple_loop[i] = 2;
early) and in this function check it for both loop_n and i.

> +	if (t == t2)
> +	  {
> +	    HOST_WIDE_INT intval;
> +	    if (gfc_extract_hwi (code->ext.iterator->step, &intval, 0) == 0
> +		&& intval != 1 && intval != -1)
> +	      sorry_at (gfc_get_location (&code->loc),
> +			"non-rectangular loop nest with non-unit loop iteration"
> +			" step for %qs", var->name);

I'd say step other than constant 1 or -1.

> +  ! Use 'i' or 'j', unite stride on 'i' or on 'j' -> 4 loops

unit ?

> +  ! Then same, execpt use nonunit stride for 'k'

except, non-unit ?

> +  ! Use 'i' or 'j', unite stride on 'i' or on 'j' -> 4 loops
> +  ! Then same, execpt use nonunit stride for 'k'

2x again
(and some more later).

	Jakub