Re: [stage1][PATCH] Lower VEC_COND_EXPR into internal functions.

["Martin Liška" <mliska@suse.cz>]

On 6/9/20 3:42 PM, Richard Biener wrote:
> On Mon, Jun 8, 2020 at 1:04 PM Martin Liška <mliska@suse.cz> wrote:
>>
>> Hello.
>>
>> Thank you for the approval. There's the patch that defines 4 new DEF_INTERNAL_OPTAB_FN.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>> It also builds on ppc64le-linux-gnu.
>>
>> Ready to be installed?
> 
> The ChangeLog refers to DEF_INTERNAL_OPTAB_CAN_FAIL which is no longer there.


Sure.

> 
> Can you put the isel pass to a separate file please?

Yes.

> 
> So this is a first step towards sanitizing VEC_COND_EXPR.  There were followups
> mentioned, namely a) enforcing that VEC_COND_EXPR constraint everywhere,
> b) isel vector comparisons at the same time since expansion has a
> vec_cond fallback

I'm planning to work on the follow up.

> 
> There's
> 
> +         /* ???  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;

Hm, I've tried to comment the code in both ISEL and expansion and I can't find a test-case
that would trigger a verification error (in vect.exp and i386.exp). Can you come up with
something that will trigger the code?

> 
> which of course is bad.  It's the comparison that can throw and I guess current
> RTL expansion manages to cope by find_many_sub_bbs and friends.  But we
> need to get this correct on GIMPLE here.  Note I find it odd this only triggers
> during ISEL - it should trigger during the lowering step which splits
> the comparison
> from the VEC_COND_EXPR.  An appropriate fix at lowering time would be to
> insert the VEC_COND_EXPR w/o the condition on the normal outgoing edge
> and keep the comparison in place of the original VEC_COND_EXPR,
> moving EH info from the VEC_COND_EXPR to the comparison.

Ah ok, so it's about correction of EH..

Martin

> 
> I think we need to fix that before merging.
> 
> Thanks,
> Richard.
> 
>> Thanks,
>> Martin