public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <rguenther@suse.de>
To: Robin Dapp <rdapp.gcc@gmail.com>
Cc: Tamar Christina <Tamar.Christina@arm.com>,
	 gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] ifcvt/vect: Emit COND_ADD for conditional scalar reduction.
Date: Mon, 9 Oct 2023 13:05:48 +0000 (UTC)	[thread overview]
Message-ID: <nycvar.YFH.7.77.849.2310091255210.5561@jbgna.fhfr.qr> (raw)
In-Reply-To: <5fc554a1-fb0c-ebcc-cfae-6b714d3aca38@gmail.com>

On Mon, 9 Oct 2023, Robin Dapp wrote:

> > Hmm, the function is called at transform time so this shouldn't help
> > avoiding the ICE.  I expected we refuse to vectorize _any_ reduction
> > when sign dependent rounding is in effect?  OTOH maybe sign-dependent
> > rounding is OK but only when we use a unconditional fold-left
> > (so a loop mask from fully masking is OK but not an original COND_ADD?).
> 
> So we currently only disable the use of partial vectors
> 
>       else if (reduction_type == FOLD_LEFT_REDUCTION
> 	       && reduc_fn == IFN_LAST

aarch64 probably chokes because reduc_fn is not IFN_LAST.

> 	       && FLOAT_TYPE_P (vectype_in)
> 	       && HONOR_SIGNED_ZEROS (vectype_in)

so with your change we'd support signed zeros correctly.

> 	       && HONOR_SIGN_DEPENDENT_ROUNDING (vectype_in))
> 	{
> 	  if (dump_enabled_p ())
> 	    dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> 			     "can't operate on partial vectors because"
> 			     " signed zeros cannot be preserved.\n");
> 	  LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P (loop_vinfo) = false;
> 
> which is inside a LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P block.
> 
> For the fully masked case we continue (and then fail the assertion
> on aarch64 at transform time).
> 
> I didn't get why that case is ok, though?  We still merge the initial
> definition with the identity/neutral op (i.e. possibly -0.0) based on
> the loop mask.  Is that different to partial masking?

I think the main point with my earlier change is that without
native support for a fold-left reduction (like on x86) we get

 ops = mask ? ops : neutral;
 acc += ops[0];
 acc += ops[1];
 ...

so we wouldn't use a COND_ADD but add neutral elements for masked
elements.  That's OK for signed zeros after your change (great)
but not OK for sign dependent rounding (because we can't decide on
the sign of the neutral zero then).

For the case of using an internal function, thus direct target support,
it should be OK to have sign-dependent rounding if we can use
the masked-fold-left reduction op.  As we do

      /* On the first iteration the input is simply the scalar phi
         result, and for subsequent iterations it is the output of
         the preceding operation.  */
      if (reduc_fn != IFN_LAST || (mask && mask_reduc_fn != IFN_LAST))
        {
          if (mask && len && mask_reduc_fn == IFN_MASK_LEN_FOLD_LEFT_PLUS)
            new_stmt = gimple_build_call_internal (mask_reduc_fn, 5, 
reduc_var,
                                                   def0, mask, len, bias);
          else if (mask && mask_reduc_fn == IFN_MASK_FOLD_LEFT_PLUS)
            new_stmt = gimple_build_call_internal (mask_reduc_fn, 3, 
reduc_var,
                                                   def0, mask);
          else
            new_stmt = gimple_build_call_internal (reduc_fn, 2, reduc_var,
                                                   def0);

the last case should be able to assert that 
!HONOR_SIGN_DEPENDENT_ROUNDING (also the reduc_fn == IFN_LAST case).

The quoted condition above should change to drop the HONOR_SIGNED_ZEROS
condition and the reduc_fn == IFN_LAST should change, maybe to
internal_fn_mask_index (reduc_fn) == -1?

Richard.

  reply	other threads:[~2023-10-09 13:05 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-20 13:51 Robin Dapp
2023-09-27  0:44 ` Tamar Christina
2023-10-04  7:54   ` Robin Dapp
2023-10-04 13:15     ` Robin Dapp
2023-10-04 15:12     ` Tamar Christina
2023-10-05  8:54       ` Robin Dapp
2023-10-05  9:02       ` Robin Dapp
2023-10-05 14:05       ` Robin Dapp
2023-10-05 14:15         ` Tamar Christina
2023-10-06  9:10         ` Richard Biener
2023-10-06 12:28           ` Robin Dapp
2023-10-06 12:30             ` Robin Dapp
2023-10-06 13:43               ` Richard Biener
2023-10-06 20:54                 ` Robin Dapp
2023-10-09  8:25                   ` Richard Biener
2023-10-09 12:54                     ` Robin Dapp
2023-10-09 13:05                       ` Richard Biener [this message]
2023-10-09  5:50         ` Richard Sandiford
2023-10-09 12:02           ` Robin Dapp
2023-10-09 14:57             ` Richard Sandiford
2023-10-11 19:15               ` Robin Dapp
2023-10-12 10:47                 ` Richard Sandiford
2023-10-12 11:11                 ` Richard Biener
2023-10-19 20:07                   ` Robin Dapp
2023-10-23 10:53                     ` Richard Biener
2023-10-24 11:11                       ` Richard Sandiford
2023-10-24 19:56                         ` Robin Dapp
2023-10-31 21:04                           ` Richard Sandiford
2023-10-31 21:19                             ` Robin Dapp
2023-11-02  7:48                               ` Richard Biener
2023-09-27 11:42 ` Richard Biener
2023-11-02 23:26 ` Andrew Pinski

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=nycvar.YFH.7.77.849.2310091255210.5561@jbgna.fhfr.qr \
    --to=rguenther@suse.de \
    --cc=Tamar.Christina@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rdapp.gcc@gmail.com \
    /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).