[PATCH] Fix PR37053: Move tweaks of commutative precedence to target hook

Peter Bergner bergner@vnet.ibm.com
Fri Jul 3 05:52:00 GMT 2009


On Sat, 2009-06-27 at 16:33 +0200, Paolo Bonzini wrote:
> Peter Bergner wrote:
> > What exactly did you have in mind here?
> 
> Look into each MEM looking for PLUSes with the operands in the wrong 
> order.  Or:
[snip]
> and then in a machine-dependent reorg pass (which rs6000 currently does 
> not have):

Ok, I gave this a whirl and although it bootstrapped with no errors, we
hit a few extra ICE's in the testsuite which I looked at.  The problem
is that your code didn't handle load/store with update instructions.
For example:

    (insn 545 897 898 35 (parallel [
            (set (reg/v:SF 45 13 [orig:255 t ] [255])
                (mem:SF (plus:SI (reg:SI 10 10)
                        (reg/f:SI 9 9 [621])) [2 S4 A32]))
            (set (reg:SI 10 10)
                (plus:SI (reg:SI 10 10)
                    (reg/f:SI 9 9 [621])))
        ]) 404 {*movsf_update1} (nil))

I started to add that support (ie, need to call find_plus_and_frob_them
on the address update portion of the insn), when I realized there is
a problem we cannot handle.  The problem is, the dest reg of the address
update (ie, r10) has to be allocated to the same hard reg as the first
operand, so swapping of load/store with update insns after register
allocation is not allowed.  And since r9 is the pointer, we lose.
Had we done this scan with pseudos just before register allocation, we
would have seen this insn which we could swap (because reg 621 dies
in the insn):

    (insn 545 411 546 35 (parallel [
            (set (reg/v:SF 255 [ t ])
                (mem:SF (plus:SI (reg:SI 408 [ D.2317 ])
                        (reg/f:SI 621)) [2 S4 A32]))
            (set (reg:SI 172 [ ivtmp.27 ])
                (plus:SI (reg:SI 408 [ D.2317 ])
                    (reg/f:SI 621)))
        ]) 404 {*movsf_update1} (expr_list:REG_DEAD (reg/f:SI 621)
        (nil)))

Is there a location before register allocation where we can do this pass
so we can catch these types of update insns?  In fact, I think we'd want
to do our swapping before combine which created these update insns, since
creating an update insn could disallow a swap if say reg 621 above didn't
die in insn 545.


Peter






More information about the Gcc-patches mailing list