Jakub Jelinek <ja...@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 <ubiz...@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