RFR: 8216392: Enable cmovP_mem and cmovP_memU instructions

[B. Blaser]

On Tue, 15 Jan 2019 at 12:16, Roman Kennke <rkennke at redhat.com> wrote:
>
> >> I agree with that. However, note that this is not about using cmov vs.
> >> branches. This is about generating a load followed by a cmov on the
> >> resulting register vs generating a cmov that also does the load and
> >> avoids the register. It's pretty much the same data-dependency-wise,
> >> except that it avoids using the extra register and encodes smaller.
> >
> > Sure, I get that. But, for the reasons given, CMOV is a rather dusty
> > corner of the ISA. Intel themselves recommend not using it unless you
> > know that the branch is always unpredictable. They say "Use the SETCC
> > and CMOV instructions to eliminate unpredictable conditional branches
> > where possible. Do not do this for predictable branches." It really
> > couldn't be clearer.
>
> Well yeah, but again, this patch isn't about generating cmov or not, it
> only changes that a cmov preceded by a load (mov) is generated as single
> instruction rather than two instructions for object loads, pretty much
> as it's done for all the other types. However, it's not very important
> to me, and probably anybody else, otherwise this wouldn't have been
> commented-out. I'd withdraw the patch unless somebody steps up and
> really wants it.

To answer Andrew Haley, one of the major difference between CISC and
RISC is specifically the load/store architecture of the latter which
is part of most instructions of the former; I don't see many good
reasons to generate RISC-like load/store code using only a subset of
instructions and to juggle with registers. Note also that if a
'mov+cmov' would now appear to be faster than a sole 'cmov' on some
processors, there is a high probability to see the opposite behavior
in future generations.

Of course, if you use another idiom like 'cmp+branch' which isn't the
purpose of this fix, you might have benefits for predictable branches
or not for unpredictable branches.

At my mind, It'd be unfortunate to withdraw this patch as the current
policy seems to go in Roman's direction, see 'cmovL_mem' which uses an
adequate prefix:

http://hg.openjdk.java.net/jdk/jdk/file/d3aa93570779/src/hotspot/cpu/x86/x86_64.ad#l7083

I'm not skilled enough in the pointer type area to answer Andrew
Dinn's question but maybe adding predicates to validate them if
Roman's prefix correction isn't sufficient would be a possible
solution?

In any case, explanations about the initial 'cmovP_mem' intention and
the reason for disabling it would be helpful.

Bernard