On 11/26/21 09:12, Richard Biener wrote:
> On Wed, Nov 24, 2021 at 3:32 PM Martin Liška <mliska@suse.cz> wrote:
>>
>> On 11/24/21 15:14, Martin Liška wrote:
>>> It likely miscompiles gcc.dg/loop-unswitch-5.c, working on that..
>>
>> Fixed that in the updated version.
> 
> Function level comments need updating it seems.

I've done that.

> 
> +static unsigned
> +evaluate_insns (class loop *loop,  basic_block *bbs,
> +               predicate_vector &predicate_path,
> +               auto_bb_flag &reachable_flag)
> +{
> +  auto_vec<basic_block> worklist (loop->num_nodes);
> +  worklist.quick_push (bbs[0]);
> ...
> 
> so when adding gswitch support the easiest way to make
> 
> +      FOR_EACH_EDGE (e, ei, bb->succs)
> +       {
> ...
> +           {
> +             worklist.safe_push (dest);
> +             dest->flags |= reachable_flag;
> 
> work is when the gcond/gswitch simplification would mark
> outgoing edges as (non-)executable.  For gswitch this
> could be achieved by iterating over the case labels and
> intersecting that with the range while for gcond it's a
> matter of setting an edge flag instead of returning true/false.

Exactly, it can be quite naturally added to the current patch.

> I'd call the common function evaluate_control_stmt_using_entry_checks
> or so and invoke it on the last stmt of a block with >= 2 outgoing
> edges.

Yes, I'll do it for the gswitch support patch.

> 
> We still seem to do the simplification work twice, once for costing
> and once for transform, but that's OK for now I guess.
> 
> I think you want to clear_aux_for_blocks at the end of the pass.

Called that.

> 
> Otherwise I like it - it seems you have some TODO around cost
> modeling.  Did you try to do gswitch support ontop as I suggested
> to see if the general structure keeps working?

I vanished and tested the patch. No, I don't have the gswitch support patch
as the current patch was reworked a few times.

Can we please progress and have installed the suggested patch?

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

> 
> Thanks,
> Richard.
> 
>>
>> Martin