Re: [Committed] PR middle-end/18548: RTL expansion of MAX_EXPR

[Roger Sayle <roger at eyesopen dot com>]

On Sat, 18 Dec 2004, Steven Bosscher wrote:
> On Saturday 18 December 2004 14:12, Roger Sayle wrote:
> > This patch resolves PR middle-end/18548 which is a wrong-code regression
> > on mainline.  The problem is that the RTL sequences generated for MIN_EXPR
> > and MAX_EXPR generated can clobber "target" which may still be required
> > to evaluate op0 and/or op1.
>
> Heh, you beat me to it :-)

I only noticed you'd assigned this PR to yourself after posting/committing
my patch.  In my defense, the PR wasn't assigned when Andrew Pinkia asked
me to take a look at it last night.  Sorry.


> So, what if the optab requires target for op0 and op1?  There is still this:
>
>       /* First try to do it with a special MIN or MAX instruction.
>          If that does not win, use a conditional jump to select the proper
>          value.  */
>       this_optab = optab_for_tree_code (code, type);
>       temp = expand_binop (mode, this_optab, op0, op1, target, unsignedp,
>                            OPTAB_WIDEN);
>
> and with your patch we force op1 into a reg _after_ this code.  Shouldn't
> expand_binop be allowed to assume we've already handled any evaluation
> hazards?


Both expand_binop and expand_expr are supposed to be able to handle the
case where target overlaps with op0 and/or op1.  For example, its not
unreasonable to call expand_binop with target == op0 == op1, i.e. for
code such as "x = x + x".  Indeed, allowing target to overlap with op0
and/or op1 allows the middle/backend to produce more efficient mode.
The downside is that the middle-end and backend patterns have to be
carefull whenever their operands are clobbered early.  This PR catches
a case where these necessary tests are missing.

Roger
--