On Wed, Sep 20, 2023 at 6:52 AM Robin Dapp wrote:
>
> Hi,
>
> as described in PR111401 we currently emit a COND and a PLUS expression
> for conditional reductions. This makes it difficult to combine both
> into a masked reduction statement later.
> This patch improves that by directly emitting a
On Tue, 31 Oct 2023, Robin Dapp wrote:
> >> +int
> >> +internal_fn_else_index (internal_fn fn)
> >
> > The function needs a comment, maybe:
> >
> > /* If FN is an IFN_COND_* or IFN_COND_LEN_* function, return the index of
> > the
> >argument that is used when the condition is false. Return
>> +int
>> +internal_fn_else_index (internal_fn fn)
>
> The function needs a comment, maybe:
>
> /* If FN is an IFN_COND_* or IFN_COND_LEN_* function, return the index of the
>argument that is used when the condition is false. Return -1 otherwise.
> */
>
> OK for the internal-fn* and tree
Robin Dapp writes:
> Changed as suggested. The difference to v5 is thus:
>
> + if (cond_fn_p)
> + {
> + gcall *call = dyn_cast (use_stmt);
> + unsigned else_pos
> + = internal_fn_else_index (internal_fn (op.code));
> +
> + for (unsigned int
Changed as suggested. The difference to v5 is thus:
+ if (cond_fn_p)
+ {
+ gcall *call = dyn_cast (use_stmt);
+ unsigned else_pos
+ = internal_fn_else_index (internal_fn (op.code));
+
+ for (unsigned int j = 0; j < gimple_call_nu
Richard Biener writes:
> On Thu, 19 Oct 2023, Robin Dapp wrote:
>
>> Ugh, I didn't push yet because with a rebased trunk I am
>> seeing different behavior for some riscv testcases.
>>
>> A reduction is not recognized because there is yet another
>> "double use" occurrence in check_reduction_path.
On Thu, 19 Oct 2023, Robin Dapp wrote:
> Ugh, I didn't push yet because with a rebased trunk I am
> seeing different behavior for some riscv testcases.
>
> A reduction is not recognized because there is yet another
> "double use" occurrence in check_reduction_path. I guess it's
> reasonable to l
Ugh, I didn't push yet because with a rebased trunk I am
seeing different behavior for some riscv testcases.
A reduction is not recognized because there is yet another
"double use" occurrence in check_reduction_path. I guess it's
reasonable to loosen the restriction for conditional operations
her
On Wed, 11 Oct 2023, Robin Dapp wrote:
> > It wasn't very clear, sorry, but it was the last sentence I was asking
> > for clarification on, not the other bits. Why do we want to avoid
> > generating a COND_ADD when the operand is a vectorisable call?
>
> Ah, I see, apologies. Upon thinking abou
Robin Dapp writes:
>> It wasn't very clear, sorry, but it was the last sentence I was asking
>> for clarification on, not the other bits. Why do we want to avoid
>> generating a COND_ADD when the operand is a vectorisable call?
>
> Ah, I see, apologies. Upon thinking about it a bit more (thanks)
> It wasn't very clear, sorry, but it was the last sentence I was asking
> for clarification on, not the other bits. Why do we want to avoid
> generating a COND_ADD when the operand is a vectorisable call?
Ah, I see, apologies. Upon thinking about it a bit more (thanks)
I figured this hunk is no
Robin Dapp writes:
>> It'd be good to expand on this comment a bit. What kind of COND are you
>> anticipating? A COND with the neutral op as the else value, so that the
>> PLUS_EXPR (or whatever) can remain unconditional? If so, it would be
>> good to sketch briefly how that happens, and why it
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
> 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 fro
> It'd be good to expand on this comment a bit. What kind of COND are you
> anticipating? A COND with the neutral op as the else value, so that the
> PLUS_EXPR (or whatever) can remain unconditional? If so, it would be
> good to sketch briefly how that happens, and why it's better than using
> t
On Fri, 6 Oct 2023, Robin Dapp wrote:
> > So if you think you got everything correct the patch is OK as-is,
> > I just wasn't sure - maybe the neutral_element change deserves
> > a comment as to how MINUS_EXPR is handled.
>
> Heh, I never think I got everything correct ;)
>
> Added this now:
>
Robin Dapp writes:
> Hi Tamar,
>
>> The only comment I have is whether you actually need this helper
>> function? It looks like all the uses of it are in cases you have, or
>> will call conditional_internal_fn_code directly.
> removed the cond_fn_p entirely in the attached v3.
>
> Bootstrapped and
> So if you think you got everything correct the patch is OK as-is,
> I just wasn't sure - maybe the neutral_element change deserves
> a comment as to how MINUS_EXPR is handled.
Heh, I never think I got everything correct ;)
Added this now:
static bool
fold_left_reduction_fn (code_helper code,
On Fri, 6 Oct 2023, Robin Dapp wrote:
> > We might need a similar assert
> >
> > gcc_assert (HONOR_SIGNED_ZEROS (vectype_out)
> > && !HONOR_SIGN_DEPENDENT_ROUNDING (vectype_out));?
>
> erm, obviously not that exact assert but more something like
>
> if (HONOR_SIGNED_
> We might need a similar assert
>
> gcc_assert (HONOR_SIGNED_ZEROS (vectype_out)
> && !HONOR_SIGN_DEPENDENT_ROUNDING (vectype_out));?
erm, obviously not that exact assert but more something like
if (HONOR_SIGNED_ZEROS && !HONOR_SIGN_DEPENDENT_ROUNDING...)
{
i
> ... here we probably get PLUS_EXPR for MINUS_EXPR above but IIRC
> for MINUS_EXPR the !as_initial case should return positive zero.
>
> Can you double-check?
You're referring to the canonicalization from a - CST to a + -CST so
that the neutral op would need to change with it? Argh, good point.
On Thu, 5 Oct 2023, Robin Dapp wrote:
> Hi Tamar,
>
> > The only comment I have is whether you actually need this helper
> > function? It looks like all the uses of it are in cases you have, or
> > will call conditional_internal_fn_code directly.
> removed the cond_fn_p entirely in the attached v
Hi Robin,
> -Original Message-
> From: Robin Dapp
> Sent: Thursday, October 5, 2023 3:06 PM
> To: Tamar Christina ; gcc-patches patc...@gcc.gnu.org>; Richard Biener
> Cc: rdapp@gmail.com
> Subject: Re: [PATCH] ifcvt/vect: Emit COND_ADD for conditional scalar
Hi Tamar,
> The only comment I have is whether you actually need this helper
> function? It looks like all the uses of it are in cases you have, or
> will call conditional_internal_fn_code directly.
removed the cond_fn_p entirely in the attached v3.
Bootstrapped and regtested on x86_64, aarch64 a
Ah, sorry, read your remark incorrectly. Will try again.
Regards
Robin
Hi Tamar,
> So in the
>
> if (slp_node)
> {
>
> Add something like:
>
> If (is_cond_op)
> {
> if (dump_enabled_p ())
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>"left fold reduction on SLP not supported.\n");
> return false;
>
Hi Robin,
> -Original Message-
> From: Robin Dapp
> Sent: Wednesday, October 4, 2023 8:54 AM
> To: Tamar Christina ; gcc-patches patc...@gcc.gnu.org>; Richard Biener
> Cc: rdapp@gmail.com
> Subject: Re: [PATCH] ifcvt/vect: Emit COND_ADD for conditional scalar
> + gcc_assert (code == IFN_COND_ADD || code == IFN_COND_SUB);
I forgot to add the other IFN_CONDs here before sending. So with
- gcc_assert (code == IFN_COND_ADD || code == IFN_COND_SUB);
+ gcc_assert (code == IFN_COND_ADD || code == IFN_COND_SUB
+ || code == IF
Hi Tamar,
> I can't approve but hope you don't mind the review,
Not at all, greatly appreciated.
I incorporated all your remarks apart from this:
> Isn't vec_opmask NULL for SLP? You probably need to read it from
> vec_defs for the COND_EXPR
Above that I gcc_assert (!slp_node) for the IFN_COND
On Wed, 20 Sep 2023, Robin Dapp wrote:
> Hi,
>
> as described in PR111401 we currently emit a COND and a PLUS expression
> for conditional reductions. This makes it difficult to combine both
> into a masked reduction statement later.
> This patch improves that by directly emitting a COND_ADD dur
Hi,
I can't approve but hope you don't mind the review,
> +/* Return true if this CODE describes a conditional (masked)
> +internal_fn. */
> +
> +bool
> +cond_fn_p (code_helper code)
> +{
> + if (!code.is_fn_code ())
> +return false;
> +
> + if (!internal_fn_p ((combined_fn) code))
> +
Hi,
as described in PR111401 we currently emit a COND and a PLUS expression
for conditional reductions. This makes it difficult to combine both
into a masked reduction statement later.
This patch improves that by directly emitting a COND_ADD during ifcvt and
adjusting some vectorizer code to hand
32 matches
Mail list logo
