On Wed, Jun 11, 2025 at 11:04 AM Eric Botcazou <botca...@adacore.com> wrote:
>
> > While you are at it
> >
> >               if ((code2 == BIT_AND_EXPR
> > -                  && TYPE_PRECISION (TREE_TYPE (op0)) == 1
> >                    && TREE_CODE (gimple_assign_rhs2 (second)) !=
> > INTEGER_CST)
> >                   || code2 == TRUTH_AND_EXPR)
> >
> > code2 can never be TRUTH_AND_EXPR (that doesn't exist on GIMPLE).
>
> Indeed, I can remove it.
>
> > +      if (!is_gimple_assign (stmt))
> > +       continue;
> > +      enum tree_code code = gimple_assign_rhs_code (stmt);
> > +      if (!commutative_tree_code (code))
> > +       continue;
> > +      /* Do not change operations expand_gimple_cond can turn into jumps.
> > */ +      if (TREE_CODE (TREE_TYPE (gimple_assign_lhs (stmt))) ==
> > BOOLEAN_TYPE +         && (code == TRUTH_AND_EXPR
> > +             || code == TRUTH_OR_EXPR
> > +             || code == BIT_AND_EXPR
> > +             || code == BIT_IOR_EXPR)
> >
> > likewise here for TRUTH_AND/OR_EXPR.
> >
> > I'll note the order is already "random" given we apply
> > tree_swap_operands_p during folding to canonicalize.  So what eventually
> > works for you in one case would now break things in another.
>
> Note that we reorder the operands "to expand the expensive one first".  But if
> we later change the operation to the short-circuit form, isn't that an obvious
> pessimization instead of a potential optimization?

Yes.  I think the attempt here is to anticipate TER and thus perform some
"scheduling", putting the more expensive (longer latency) insns first.  It feels
like black magic.  IMO the best thing would be to perform the expand_gimple_cond
"optimization" of rewriting things to jumpy sequences earlier and properly
represent this in the CFG (possibly before late sinking?).  I'm not
sure as to how
much sinking we'll perform on the RTL level here.

> > In addition to that, the expand_gimple_cond case only applies to defs that
> > have uses in a gcond *.
>
> I can restrict the change to the single-use-within-a-gcond case then.

Please - possibly matching the gcond expansion check for this,
bitmap_bit_p (SA.values, SSA_NAME_VERSION (op0))

Richard.

>
> --
> Eric Botcazou
>
>

Reply via email to