Hi!
On Tue, Apr 01, 2025 at 09:35:53PM -0600, Jeff Law wrote:
> Segher -- there's a combine question near the end...
> So this is a nasty little problem and touches on a deeper issue.
>
> The core problem is that combine doesn't know anything about
> constraints. It works with predicates and conditions. So combine has
> no idea if it's creating an ASM with operands that can't be handled. As
> a result combine has to be careful with ASMs.
If whatever combine produces passes recog(), and the insn_cost for it
is less than what it started with (or the same cost is fine as well in
many cases), it will allow it.
The result is semantically equivalent, _by construction_. The only
thing combine (or the target backend) has to be worried about is if
this is a transformation that is beneficial; it always is correct.
> In can_combine_p we have:
>
> > /* Can't merge an ASM_OPERANDS. */
> > || GET_CODE (src) == ASM_OPERANDS
>
> Which is part of a large conditional indicating when we can't combine
> two insns together.
Yes, there is no way to validly create a new extended asm (what would
the template be, to start with!), and there should not be asm_operand's
anywhere else.
> So I would have expected it to reject this insn for
> combining:
>
> >(insn 12 11 0 2 (set (mem/f:DI (reg/v/f:DI 138 [ e ]) [2 *e_6+0 S8 A64])
> > (asm_operands:DI ("") ("=A") 0 [
> > (mem/f:DI (reg/v/f:DI 138 [ e ]) [2 *e_6+0 S8 A64])
> > ]
> > [
> > (asm_input:DI ("A") j.c:8)
> > ]
> > [] j.c:8)) "j.c":8:3 -1
> > (expr_list:REG_DEAD (reg/v/f:DI 138 [ e ])
> > (nil)))
>
> So the natural question is why did insn 12 participate in combination at
> all:
>
> >Trying 11 -> 12:
> > 11: r138:DI=r145:DI+r144:DI
> > REG_DEAD r145:DI
> > REG_DEAD r144:DI
> > 12: [r138:DI]=asm_operands
> > REG_DEAD r138:DI
> >Failed to match this instruction:
> >(set (mem/f:DI (plus:DI (reg/f:DI 145 [ b ])
> > (reg:DI 144 [ _4 ])) [2 *e_6+0 S8 A64])
> > (asm_operands:DI ("") ("=A") 0 [
> > (mem/f:DI (plus:DI (reg/f:DI 145 [ b ])
> > (reg:DI 144 [ _4 ])) [2 *e_6+0 S8 A64])
> > ]
> > [
> > (asm_input:DI ("A") j.c:8)
> > ]
> > [] j.c:8))
> >allowing combination of insns 11 and 12
> >original costs 4 + 36 = 40
> >replacement cost 36
> >deferring deletion of insn with uid = 11.
> >modifying insn i3 12: [r145:DI+r144:DI]=asm_operands
> > REG_DEAD r144:DI
> > REG_DEAD r145:DI
> >deferring rescan insn with uid = 12.
> So without a deep dive, my question is should this have been rejected
> for combination before we even got here? I just don't see how combine
> can do anything with asms other than replacing one pseudo with another
> since combine doesn't have any notion of constraints.
combine already rejects extended asm (as well as basic asm) in many
places, I wonder how this got through. Was stuff in the instruction
stream already malformed, perhaps?
But, also, what is wrong here? combine is not combining anything in the
asm, just in the argument of an asm operand, and that looks perfectly
fine?
Segher
