public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Tamar Christina <Tamar.Christina@arm.com>
To: Richard Biener <rguenther@suse.de>,
	"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: RE: [PATCH] tree-optimization/111950 - vectorizer loop copying
Date: Thu, 9 Nov 2023 09:09:51 +0000	[thread overview]
Message-ID: <AM0PR08MB53168CB0FC40286C54C54DFEFFAFA@AM0PR08MB5316.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <33fb3bb3-9548-4867-95f6-f7319bde2270@AMS1EPF00000043.eurprd04.prod.outlook.com>

>  	  guard_bb = LOOP_VINFO_IV_EXIT (loop_vinfo)->dest;
>  	  edge epilog_e = LOOP_VINFO_EPILOGUE_IV_EXIT (loop_vinfo);
> -	  guard_to = split_edge (epilog_e);
> +	  guard_to = epilog_e->dest;
>  	  guard_e = slpeel_add_loop_guard (guard_bb, guard_cond, guard_to,
>  					   skip_vector ? anchor : guard_bb,
>  					   prob_epilog.invert (),
> @@ -3443,8 +3229,30 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree
> niters, tree nitersm1,
>  	  if (vect_epilogues)
>  	    epilogue_vinfo->skip_this_loop_edge = guard_e;
>  	  edge main_iv = LOOP_VINFO_IV_EXIT (loop_vinfo);
> -	  slpeel_update_phi_nodes_for_guard2 (loop, epilog, main_iv,
> guard_e,
> -					      epilog_e);
> +	  gphi_iterator gsi2 = gsi_start_phis (main_iv->dest);
> +	  for (gphi_iterator gsi = gsi_start_phis (guard_to);
> +	       !gsi_end_p (gsi); gsi_next (&gsi))
> +	    {
> +	      /* We are expecting all of the PHIs we have on epilog_e
> +		 to be also on the main loop exit.  But sometimes
> +		 a stray virtual definition can appear at epilog_e
> +		 which we can then take as the same on all exits,
> +		 we've removed the LC SSA PHI on the main exit before
> +		 so we wouldn't need to create a loop PHI for it.  */
> +	      if (virtual_operand_p (gimple_phi_result (*gsi))
> +		  && (gsi_end_p (gsi2)
> +		      || !virtual_operand_p (gimple_phi_result (*gsi2))))
> +		add_phi_arg (*gsi,
> +			     gimple_phi_arg_def_from_edge (*gsi, epilog_e),
> +			     guard_e, UNKNOWN_LOCATION);
> +	      else
> +		{
> +		  add_phi_arg (*gsi, gimple_phi_result (*gsi2), guard_e,
> +			       UNKNOWN_LOCATION);
> +		  gsi_next (&gsi2);
> +		}
> +	    }
> +

I've been having some trouble incorporating this change into the early break work.
My understanding is that here you've removed the lookup that find_guard did
and are assuming that the order between the PHI nodes between loop->exit
and epilog->exit are the same - sporadic virtual operands.

But the loop->exit for early break has to materialize all PHI nodes from the main
loop into the epilog loop since we need them to restart the scalar loop iteration.

This means that the number of PHI nodes between the first loop and the second
Loop are not the same, so we end up mis-linking phi nodes.  i.e. consider this loop

https://gist.github.com/Mistuke/65d476b18f991772fdec159a09b81869

which now goes wrong (throw that in a dotgraph viewer).

I'm struggling to figure out how to handle this without doing a lookup.

Any advice?

Thanks,
Tamar


>  	  /* Only need to handle basic block before epilog loop if it's not
>  	     the guard_bb, which is the case when skip_vector is true.  */
>  	  if (guard_bb != bb_before_epilog)
> --
> 2.35.3

       reply	other threads:[~2023-11-09  9:10 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <33fb3bb3-9548-4867-95f6-f7319bde2270@AMS1EPF00000043.eurprd04.prod.outlook.com>
2023-11-09  9:09 ` Tamar Christina [this message]
2023-11-09  9:23   ` Richard Biener
2023-11-09 10:26     ` Tamar Christina
2023-11-09 11:54       ` Richard Biener
2023-11-09 16:22         ` Tamar Christina
2023-11-10 10:21           ` Richard Biener
2023-11-06 13:14 Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM0PR08MB53168CB0FC40286C54C54DFEFFAFA@AM0PR08MB5316.eurprd08.prod.outlook.com \
    --to=tamar.christina@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rguenther@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).