Re: [PATCH v2, rtl-optimization]: Fix PR54457, [x32] Fail to combine 64bit index + constant

[Richard Sandiford <rdsandiford@googlemail.com>]

Jakub Jelinek <jakub@redhat.com> writes:
> On Thu, Sep 27, 2012 at 08:04:58PM +0200, Uros Bizjak wrote:
>> After some off-line discussion with Richard, attached is v2 of the patch.
>> 
>> 2012-09-27  Uros Bizjak  <ubizjak@gmail.com>
>> 
>>         PR rtl-optimization/54457
>>         * simplify-rtx.c (simplify_subreg):
>> 	Simplify (subreg:SI (op:DI ((x:DI) (y:DI)), 0)
>>      	to (op:SI (subreg:SI (x:DI) 0) (subreg:SI (x:DI) 0)).
>
> Is that a good idea even for WORD_REGISTER_OPERATIONS targets?

I think so.  Admittedly it means I'm going to have to change the
mips.md BADDU patterns from:

(define_insn "*baddu_si_el"
  [(set (match_operand:SI 0 "register_operand" "=d")
        (zero_extend:SI
	 (subreg:QI
	  (plus:SI (match_operand:SI 1 "register_operand" "d")
		   (match_operand:SI 2 "register_operand" "d")) 0)))]
  "ISA_HAS_BADDU && !BYTES_BIG_ENDIAN"
  "baddu\\t%0,%1,%2"
  [(set_attr "alu_type" "add")])

to:

(define_insn "*baddu_si_el"
  [(set (match_operand:SI 0 "register_operand" "=d")
        (zero_extend:SI
	 (plus:QI (match_operand:QI 1 "register_operand" "d")
		  (match_operand:QI 2 "register_operand" "d"))))]
  "ISA_HAS_BADDU"
  "baddu\\t%0,%1,%2"
  [(set_attr "alu_type" "add")])

But really, that's a better representation even on MIPS,
not least because it gets rid of the ugly endianness condition.

There will probably be fallout on other targets too.
But the upside is that we get rid of more subregs from .mds.
I think a few of the i386.md define_peephole2s could go too.
E.g. the second two of:

(define_peephole2
  [(set (match_operand:DI 0 "register_operand")
  	(zero_extend:DI
	  (plus:SI (match_operand:SI 1 "register_operand")
		   (match_operand:SI 2 "nonmemory_operand"))))]
  "TARGET_64BIT && !TARGET_OPT_AGU
   && REGNO (operands[0]) == REGNO (operands[1])
   && peep2_regno_dead_p (0, FLAGS_REG)"
  [(parallel [(set (match_dup 0)
		   (zero_extend:DI (plus:SI (match_dup 1) (match_dup 2))))
	      (clobber (reg:CC FLAGS_REG))])])

(define_peephole2
  [(set (match_operand:DI 0 "register_operand")
  	(zero_extend:DI
	  (plus:SI (match_operand:SI 1 "nonmemory_operand")
		   (match_operand:SI 2 "register_operand"))))]
  "TARGET_64BIT && !TARGET_OPT_AGU
   && REGNO (operands[0]) == REGNO (operands[2])
   && peep2_regno_dead_p (0, FLAGS_REG)"
  [(parallel [(set (match_dup 0)
		   (zero_extend:DI (plus:SI (match_dup 2) (match_dup 1))))
	      (clobber (reg:CC FLAGS_REG))])])

(define_peephole2
  [(set (match_operand:DI 0 "register_operand")
  	(zero_extend:DI
	  (subreg:SI (plus:DI (match_dup 0)
			      (match_operand:DI 1 "nonmemory_operand")) 0)))]
  "TARGET_64BIT && !TARGET_OPT_AGU
   && peep2_regno_dead_p (0, FLAGS_REG)"
  [(parallel [(set (match_dup 0)
		   (zero_extend:DI (plus:SI (match_dup 2) (match_dup 1))))
	      (clobber (reg:CC FLAGS_REG))])]
{
  operands[1] = gen_lowpart (SImode, operands[1]);
  operands[2] = gen_lowpart (SImode, operands[0]);
})

(define_peephole2
  [(set (match_operand:DI 0 "register_operand")
  	(zero_extend:DI
	  (subreg:SI (plus:DI (match_operand:DI 1 "nonmemory_operand")
		     	      (match_dup 0)) 0)))]
  "TARGET_64BIT && !TARGET_OPT_AGU
   && peep2_regno_dead_p (0, FLAGS_REG)"
  [(parallel [(set (match_dup 0)
		   (zero_extend:DI (plus:SI (match_dup 2) (match_dup 1))))
	      (clobber (reg:CC FLAGS_REG))])]
{
  operands[1] = gen_lowpart (SImode, operands[1]);
  operands[2] = gen_lowpart (SImode, operands[0]);
})

where we should now always generate the first two forms.

There's no semantic difference between the two rtxes, and I think
it would be confusing to have different canonical forms on different
targets.  If the caller really wants a word-mode operation on
WORD_REGISTER_OPERATIONS targets, then I think it's asking for
the wrong thing by taking this subreg.

Richard