Re: Re: [PATCH V4] VECT: Add decrement IV iteration loop control by variable amount support

[钟居哲]

I am sorry that I am still confused about that.

Is this what you want ?

  bool use_minus_p = TREE_CODE (step) == INTEGER_CST && ((TYPE_UNSIGNED 
(TREE_TYPE (step)) && tree_int_cst_lt (step1, step))
                     || (!TYPE_UNSIGNED (TREE_TYPE (step)) && 
!tree_expr_nonnegative_warnv_p (step, &ovf) && may_negate_without_overflow_p 
(step)));

  /* For easier readability of the created code, produce MINUS_EXPRs
     when suitable.  */
  if (TREE_CODE (step) == INTEGER_CST)
    {
      if (TYPE_UNSIGNED (TREE_TYPE (step)))
{
  step1 = fold_build1 (NEGATE_EXPR, TREE_TYPE (step), step);
  if (tree_int_cst_lt (step1, step))
    {
      incr_op = MINUS_EXPR; /* Remove it.  */
      step = step1;
    }
}
      else
{
  bool ovf;

  if (!tree_expr_nonnegative_warnv_p (step, &ovf)
      && may_negate_without_overflow_p (step))
    {
      incr_op = MINUS_EXPR; /* Remove it.  */
      step = fold_build1 (NEGATE_EXPR, TREE_TYPE (step), step);
    }
}
    }
  if (POINTER_TYPE_P (TREE_TYPE (base)))
    {
      if (TREE_CODE (base) == ADDR_EXPR)
mark_addressable (TREE_OPERAND (base, 0));
      step = convert_to_ptrofftype (step);
      if (incr_op == MINUS_EXPR) /* Change it into if (use_minus_p)  */
step = fold_build1 (NEGATE_EXPR, TREE_TYPE (step), step);
      incr_op = POINTER_PLUS_EXPR; /* Remove it.  */
    }
  /* Gimplify the step if necessary.  We put the computations in front of the
     loop (i.e. the step should be loop invariant).  */
  step = force_gimple_operand (step, &stmts, true, NULL_TREE);
  if (stmts)
    gsi_insert_seq_on_edge_immediate (pe, stmts);
  
  if (POINTER_TYPE_P (TREE_TYPE (base)))
    stmt = gimple_build_assign (va, POINTER_PLUS_EXPR, vb, step);
  else if (use_minus_p)
    stmt = gimple_build_assign (va, MINUS_EXPR, vb, step);
  else
    stmt = gimple_build_assign (va, incr_op, vb, step);
...

Since I have no idea to make stmts flips between PLUS_EXPR and MINUS_EXPR.

Thanks.


juzhe.zh...@rivai.ai
 
From: Richard Sandiford
Date: 2023-05-11 05:28
To: 钟居哲
CC: gcc-patches; rguenther
Subject: Re: [PATCH V4] VECT: Add decrement IV iteration loop control by 
variable amount support
钟居哲 <juzhe.zh...@rivai.ai> writes:
> Thanks Richard.
> I am planning to seperate a patch with only creat_iv stuff only.
>
> Are you suggesting that I remove "tree_code incr_op = code;"
> Use the argument directly ?
>
> I saw the codes here:
>
>   /* For easier readability of the created code, produce MINUS_EXPRs
>      when suitable.  */
>   if (TREE_CODE (step) == INTEGER_CST)
>     {
>       if (TYPE_UNSIGNED (TREE_TYPE (step)))
> {
>   step1 = fold_build1 (NEGATE_EXPR, TREE_TYPE (step), step);
>   if (tree_int_cst_lt (step1, step))
>     {
>       incr_op = MINUS_EXPR;
>       step = step1;
>     }
> }
>       else
> {
>   bool ovf;
>
>   if (!tree_expr_nonnegative_warnv_p (step, &ovf)
>       && may_negate_without_overflow_p (step))
>     {
>       incr_op = MINUS_EXPR;
>       step = fold_build1 (NEGATE_EXPR, TREE_TYPE (step), step);
>     }
> }
>     }
>   if (POINTER_TYPE_P (TREE_TYPE (base)))
>     {
>       if (TREE_CODE (base) == ADDR_EXPR)
> mark_addressable (TREE_OPERAND (base, 0));
>       step = convert_to_ptrofftype (step);
>       if (incr_op == MINUS_EXPR)
> step = fold_build1 (NEGATE_EXPR, TREE_TYPE (step), step);
>       incr_op = POINTER_PLUS_EXPR;
>     }
>   /* Gimplify the step if necessary.  We put the computations in front of the
>      loop (i.e. the step should be loop invariant).  */
>   step = force_gimple_operand (step, &stmts, true, NULL_TREE);
>   if (stmts)
>     gsi_insert_seq_on_edge_immediate (pe, stmts);
>
>   stmt = gimple_build_assign (va, incr_op, vb, step);
> ...
>
> It seems that it has complicated conditions here to change value of variable 
> "incr_op".
> That's why I define a temporary variable "tree_code incr_op = code;" here and
> let the following codes change the value of "incr_op".
>
> Could you give me some hints of dealing with this piece of code to get rid of 
> "tree_code incr_op = code;" ?
 
Yeah, but like I said in the review, those later:
      incr_op = MINUS_EXPR;
stmts need to be updated to something that flips between PLUS_EXPR
and MINUS_EXPR (with updates to the comments).  Just leaving them
as-is is incorrect (in cases where the caller passed MINUS_EXPR
rather than PLUS_EXPR).
 
The POINTER_PLUS_EXPR handling is fine due to the conditional
negate beforehand.
 
Thanks,
Richard