On Fri, Nov 27, 2020 at 4:07 PM Martin Liška <mli...@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 <mli...@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
>
