On Wed, 10 May 2017, Jakub Jelinek wrote:
> On Wed, May 10, 2017 at 09:57:56PM +0200, Uros Bizjak wrote:
> > BTW: This patch now catches 417 cases (instead of 200+) in linux
> > build, including e.g.:
> >
> > (parallel [
> > (set (reg:CCZ 17 flags)
> > (compare:CCZ (lshiftrt:SI (reg:SI 4 si [orig:93 _10 ] [93])
> > (const_int 1 [0x1]))
> > (const_int 0 [0])))
> > (set (reg:DI 4 si)
> > (zero_extend:DI (lshiftrt:SI (reg:SI 4 si [orig:93 _10 ] [93])
> > (const_int 1 [0x1]))))
> > ])
>
> That looks nice. So, I think we need analysis on what order which targets
> use. I have looked at mn10300.md, I see {add,sub}si3_flags patterns that
> would need PARALLEL reordering for this compare-elim.c change and then
> cmp_liw vs. liw_cmp patterns I have no clue what they do and whether
> compare-elim.c would care about those or not (they have UNSPECs). Jeff/Alex?
>
> In rx.md I see {add,sub}si3_flags too, then ssaddsi3 and 2 peepholes that
> would need changing.
The canonical order of insns affecting condition-codes has been
discussed in the past too.
I was then arguing that the compare should go last, i.e.
(parallel [(set (reg) (op...))
(set (ccreg) (compare (op...) (const_int 0)))])
for simplicity of processing together with an alternative that
clobbers (i.e. same location in the parallel), as the canonical
order of clobbers in a parallel is that they always go last.
(IIRC from that distant past, transforming a cc0 target to
"ccreg" required at least three variants: the "naked" insn
(pre-reload?), the parallel with a clobber of ccreg, and the
actual use of ccreg; as above.)
Anyway, people seem to drift towards the ccreg-last variant for
some reason or another every time this is brought up; this time
it seems the single reason is that some existing pass expects or
generates this, using the order that causes the minimal fallout.
(Maybe it's the same pass...) Not sure that's a good base for
decisions.
Whatever you do in the end, *please document the canonical
order* in the RTL documentation.
brgds, H-P
