[PATCH v2] Add if-chain to switch conversion pass.

[Richard Biener]

On Fri, Nov 27, 2020 at 4:07 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 11/20/20 3:37 PM, Richard Biener wrote:
> > On Fri, Nov 20, 2020 at 9:57 AM Martin Liška <mliska@suse.cz> wrote:
> >>
> >> On 11/19/20 3:46 PM, Richard Biener wrote:
> >>> OK, so can you send an updated patch?
> >
> > +      tree pos_one = build_int_cst (type, 1);
> > +      if (!left->m_has_forward_bb
> > +         && !right->m_has_forward_bb
> > +         && left->m_case_bb == right->m_case_bb)
> > +       {
> > +         tree next = int_const_binop (PLUS_EXPR, left->get_high (), pos_one);
> > +         if (tree_int_cst_equal (next, right->get_low ()))
> >
> > can you please avoid tree arithmetic here and use wide_ints?
> >
> >    if (wi::eq (wi::to_wide (right->get_low) - wi::to_wide
> > (left->get_high), wi::one (TYPE_PRECISION (type))
> >
> > ?
> >
> > +      info.record_phi_mapping (info.m_true_edge,
> > +                              &info.m_true_edge_phi_mapping);
> > +      info.record_phi_mapping (info.m_false_edge,
> > +                              &info.m_false_edge_phi_mapping);
> >
> > you still have this edge mapping stuff, can't you recover the actual
> > PHI arguments from the PHIs during code generation?  I see you removed
> > the restriction for all-same values, good.
> >
> > +unsigned int
> > +pass_if_to_switch::execute (function *fun)
> > +{
> > +  auto_vec<if_chain *> all_candidates;
> > +  hash_map<basic_block, condition_info> conditions_in_bbs;
> > +
> > +  basic_block bb;
> > +  FOR_EACH_BB_FN (bb, fun)
> > +    find_conditions (bb, &conditions_in_bbs);
> > +
> >
> > if we didn't find any suitable conditions we can early out
> >
> > +  free_dominance_info (CDI_DOMINATORS);
> > +
> > +  if (!all_candidates.is_empty ())
> > +    mark_virtual_operands_for_renaming (fun);
> >
> > please do not free dominator info when you did nothing
> > (all_candidates.is_empty ()).
> >
> > +             gcond *cond = chain->m_entries[0]->m_cond;
> > +             expanded_location loc = expand_location (gimple_location (cond));
> > +             if (dump_file)
> > +               {
> > +                 fprintf (dump_file, "Condition chain (at %s:%d) with %d BBs "
> > +                          "transformed into a switch statement.\n",
> > +                          loc.file, loc.line,
> > +                          chain->m_entries.length ());
> > +               }
> >
> > if you use dump_enabled_p () and dump_printf_loc you can
> > use 'cond' as location itself.
> >
> > Otherwise looks OK.
>
> Hello.
>
> I fixed all the notes except the PHI mapping map. It's really not simple
> to reconstruct PHI nodes during code-gen. Once a BB is removed in transform phase,
> I would need to investigate which PHIs have "holes" (missing PHI argument).
>
> Moreover, I do the very similar thing in gcc/tree-switch-conversion.c::record_phi_operand_mapping.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> And I run SPEC2006 benchmarks, it has done 413 transformations.

OK.

Thanks,
Richard.

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