[stage1][PATCH] Lower VEC_COND_EXPR into internal functions.
Martin Liška
mliska@suse.cz
Mon Jun 15 11:19:18 GMT 2020
On 6/15/20 9:14 AM, Richard Biener wrote:
> On Fri, Jun 12, 2020 at 3:24 PM Martin Liška <mliska@suse.cz> wrote:
>>
>> On 6/12/20 11:43 AM, Richard Biener wrote:
>>> So ... how far are you with enforcing a split VEC_COND_EXPR?
>>> Thus can we avoid the above completely (even as intermediate
>>> state)?
>>
>> Apparently, I'm quite close. Using the attached patch I see only 2 testsuite
>> failures:
>>
>> FAIL: gcc.dg/tree-ssa/pr68714.c scan-tree-dump-times reassoc1 " <= " 1
>> FAIL: gcc.target/i386/pr78102.c scan-assembler-times pcmpeqq 3
>>
>> The first one is about teaching reassoc about the SSA_NAMEs in VEC_COND_EXPR. I haven't
>> analyze the second failure.
>>
>> I'm also not sure about the gimlification change, I see a superfluous assignments:
>> vec_cond_cmp.5 = _1 == _2;
>> vec_cond_cmp.6 = vec_cond_cmp.5;
>> vec_cond_cmp.7 = vec_cond_cmp.6;
>> _3 = VEC_COND_EXPR <vec_cond_cmp.7, { -1, -1, -1, -1, -1, -1, -1, -1 }, { 0, 0, 0, 0, 0, 0, 0, 0 }>;
>> ?
>>
>> So with the suggested patch, the EH should be gone as you suggested. Right?
>
> Right, it should be on the comparison already from the start.
>
> @@ -14221,9 +14221,13 @@ gimplify_expr (tree *expr_p, gimple_seq
> *pre_p, gimple_seq *post_p,
> case VEC_COND_EXPR:
> {
> enum gimplify_status r0, r1, r2;
> -
> r0 = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p,
> post_p, is_gimple_condexpr, fb_rvalue);
> + tree xop0 = TREE_OPERAND (*expr_p, 0);
> + tmp = create_tmp_var_raw (TREE_TYPE (xop0), "vec_cond_cmp");
> + gimple_add_tmp_var (tmp);
> + gimplify_assign (tmp, xop0, pre_p);
> + TREE_OPERAND (*expr_p, 0) = tmp;
> r1 = gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p,
> post_p, is_gimple_val, fb_rvalue);
>
> all of VEC_COND_EXPR can now be a simple goto expr_3;
Works for me, thanks!
>
> diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c
> index 494c9e9c20b..090fb52a2f1 100644
> --- a/gcc/tree-ssa-forwprop.c
> +++ b/gcc/tree-ssa-forwprop.c
> @@ -3136,6 +3136,10 @@ pass_forwprop::execute (function *fun)
> if (code == COND_EXPR
> || code == VEC_COND_EXPR)
> {
> + /* Do not propagate into VEC_COND_EXPRs. */
> + if (code == VEC_COND_EXPR)
> + break;
> +
>
> err - remove the || code == VEC_COND_EXPR instead?
Yep.
>
> @@ -2221,24 +2226,12 @@ expand_vector_operations (void)
> {
> gimple_stmt_iterator gsi;
> basic_block bb;
> - bool cfg_changed = false;
>
> FOR_EACH_BB_FN (bb, cfun)
> - {
> - for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
> - {
> - expand_vector_operations_1 (&gsi);
> - /* ??? If we do not cleanup EH then we will ICE in
> - verification. But in reality we have created wrong-code
> - as we did not properly transition EH info and edges to
> - the piecewise computations. */
> - if (maybe_clean_eh_stmt (gsi_stmt (gsi))
> - && gimple_purge_dead_eh_edges (bb))
> - cfg_changed = true;
> - }
> - }
>
> I'm not sure about this. Consider the C++ testcase where
> the ?: is replaced by a division. If veclower needs to replace
> that with four scalrar division statements then the above
> still applies - veclower does not correctly duplicate EH info
> and EH edges to the individual divisions (and we do not know
> which component might trap).
>
> So please leave the above in. You can try if using integer
> division makes it break and add such a testcase if there's
> no coverage for this in the testsuite.
I'm leaving that above. Can you please explain how can a division test-case
be created?
>
> What's missing from the patch is adjusting
> verify_gimple_assign_ternary from
>
> if (((rhs_code == VEC_COND_EXPR || rhs_code == COND_EXPR)
> ? !is_gimple_condexpr (rhs1) : !is_gimple_val (rhs1))
> || !is_gimple_val (rhs2)
> || !is_gimple_val (rhs3))
> {
> error ("invalid operands in ternary operation");
> return true;
>
> to the same with the rhs_code == VEC_COND_EXPR case removed.
Hmm. I'm not sure I've got this comment. Why do we want to change it
and is it done wright in the patch?
>
> You'll likely figure the vectorizer still creates some VEC_COND_EXPRs
> with embedded comparisons.
I've fixed 2 failing test-cases I mentioned in the previous email.
Martin
>
> Thanks,
> Richard.
>
>
>> Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Lower-VEC_COND_EXPR-into-internal-functions.patch
Type: text/x-patch
Size: 37075 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20200615/e01056b5/attachment-0001.bin>
More information about the Gcc-patches
mailing list