On Fri, 2012-08-17 at 22:32 +0200, Uros Bizjak wrote:
> Yes, x86_64 also has register passing convention. So, i.e.:
>
> --cut here--
> int foo (int a, int b, int c)
> {
> return a + b + c;
> }
> --cut here--
>
> expands to:
>
> (note 6 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
> (insn 2 6 3 2 (set (reg/v:SI 62 [ a ])
> (reg:SI 5 di [ a ])) t.c:2 -1
> (nil))
> (insn 3 2 4 2 (set (reg/v:SI 63 [ b ])
> (reg:SI 4 si [ b ])) t.c:2 -1
> (nil))
> (insn 4 3 5 2 (set (reg/v:SI 64 [ c ])
> (reg:SI 1 dx [ c ])) t.c:2 -1
> (nil))
> (note 5 4 8 2 NOTE_INSN_FUNCTION_BEG)
> (insn 8 5 9 2 (parallel [
> (set (reg:SI 66 [ D.1722 ])
> (plus:SI (reg/v:SI 62 [ a ])
> (reg/v:SI 63 [ b ])))
> (clobber (reg:CC 17 flags))
> ]) t.c:3 -1
> (nil))
> (...)
>
> So, we expand function arguments to pseudos first, and these survive
> up to the combine pass. If sh expanded func arguments to pseudos, then
> invalid hard register won't be propagated to insn pattern, leaving
> reload with much more freedom to choose correct register. I believe
> that code improvements you are seeing come from this added reload
> freedom.
It seems on SH func args are expanded to pseudos, sorry for not checking
this properly.
int test (uint8_t x)
{
return x & 15 ? -40 : -9;
}
expands to:
(note 1 0 7 NOTE_INSN_DELETED)
(note 7 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(insn 2 7 3 2 (set (reg:SI 164)
(reg:SI 4 r4 [ x ])) sh_tmp.cpp:9 -1
(nil))
(insn 3 2 4 2 (set (reg/v:SI 163 [ x ])
(zero_extend:SI (subreg:QI (reg:SI 164) 0))) sh_tmp.cpp:9 -1
(nil))
(note 4 3 9 2 NOTE_INSN_FUNCTION_BEG)
(insn 9 4 10 2 (set (reg:SI 165)
(and:SI (reg/v:SI 163 [ x ])
(const_int 15 [0xf]))) sh_tmp.cpp:10 -1
(nil))
...
Then in combine it goes like:
Successfully matched this instruction:
(set (reg/v:SI 163 [ x ])
(zero_extend:SI (reg:QI 4 r4 [ x ])))
deferring deletion of insn with uid = 2.
modifying insn i3 3 r163:SI=zero_extend(r4:QI)
REG_DEAD: r4:SI
deferring rescan insn with uid = 3.
[[note: the matched pattern above is "*zero_extendqisi2_compact" ]]
Trying 3 -> 9:
Successfully matched this instruction:
(set (reg:SI 165)
(and:SI (reg:SI 4 r4 [ x ])
(const_int 15 [0xf])))
deferring deletion of insn with uid = 3.
modifying insn i3 9 r165:SI=r4:SI&0xf
REG_DEAD: r4:SI
deferring rescan insn with uid = 9.
Trying 9 -> 11:
Successfully matched this instruction:
(set (reg:SI 167)
(and:SI (reg:SI 4 r4 [ x ])
(const_int 15 [0xf])))
deferring deletion of insn with uid = 9.
modifying insn i3 11 r167:SI=r4:SI&0xf
REG_DEAD: r4:SI
deferring rescan insn with uid = 11.
Trying 11 -> 12:
Successfully matched this instruction:
(set (reg:SI 147 t)
(eq:SI (zero_extract:SI (reg:SI 4 r4 [ x ])
(const_int 4 [0x4])
(const_int 0 [0]))
(const_int 0 [0])))
Operand failed to match constraints:
(reg:SI 4 r4 [ x ])
> Hm, I'd write this instruction as:
>
> (define_insn_and_split "nott"
> [(set (match_operand:SI 0 "t_reg_operand" "t")
> (xor:SI (match_operand:SI 1 "t_reg_operand" "0") (const_int 1)))]
> "TARGET_SH1"
> ...)
>
> This would match new combine limitation without problems, although
> empty constraint should be matched as well. Can you perhaps
> investigate a bit, why op_alt[j].anything.ok in
>
> + if (op_alt[j].anything_ok
> + || (op_alt[j].matches != -1
> + && rtx_equal_p (recog_data.operand[i],
> + recog_data.operand[op_alt[j].matches]))
> + || (reg_fits_class_p (op, op_alt[j].cl, offset, mode)))
>
> does not trigger for this operation? Maybe we need to add another
> condition to allow empty constraint.
It seems that in this case there are no alternatives in recog_data at
all. The 'for' never runs, 'win' remains 'false'.
Doing
- if (!win)
+ if (!win && recog_data.n_alternatives > 0)
seems to help.
> IMO, a patch like this that touches many targets won't be applied over
> night. Rest assured that there will be a discussion about it.
>
> Thanks for testing the patch, I look forward to comments.
Sure, no problem.
Cheers,
Oleg
