RFR: 8208171: PPC64: Enrich SLP support
Michihiro Horie
HORIE at jp.ibm.com
Wed Sep 5 10:22:57 UTC 2018
Hi Martin, Gustavo,
I cannot still reproduce the problem. I noticed the machine I have is not
SUSE but OpenSUSE with 4.1.21-14-default. I've also tried kernel
4.4.0-31-generic but it's Ubuntu.
Gustavo, is there any suspicious change before/after v4.4, which Martin got
the crash?
Apart from the problem, I uploaded the latest webrev:
http://cr.openjdk.java.net/~mhorie/8208171/webrev.03/
Best regards,
--
Michihiro,
IBM Research - Tokyo
From: Gustavo Romero <gromero at linux.vnet.ibm.com>
To: "Doerr, Martin" <martin.doerr at sap.com>, Michihiro
Horie/Japan/IBM at IBMJP
Cc: "Lindenmaier, Goetz" <goetz.lindenmaier at sap.com>, hotspot
compiler <hotspot-compiler-dev at openjdk.java.net>
Date: 2018/09/05 07:03
Subject: Re: RFR: 8208171: PPC64: Enrich SLP support
Hi Martin and Michi,
On 09/04/2018 01:20 PM, Doerr, Martin wrote:
> Can you reproduce the test failures?
>
> The very same VM works fine on a different Power8 machine which uses the
same instructions by C2.
>
> The VM was built on the machine where it works ("SUSE Linux Enterprise
Server 12 SP1").
>
> I have seen several linux kernel changes regarding saving and restoring
the VSX registers.
>
> I still haven’t found out how the kernel determines things like “tsk->
thread.used_vsr” which is used to set “msr |= MSR_VEC”.
>
> Maybe something is missing which tells the kernel that we’re using it.
But that’s just a guess.
Facilities like FP (fp registers), VEC (vector registers - aka
VMX/Altivec), and
VSX (vector-scalar registers) are usually disabled on a new born process.
Once
any instruction associated to these facilities is used in the process it
causes
an exception that is treated by the kernel [1, 2, 3]: kernel enables the
facility that caused the exception (see load_up_fp & friends) and
re-execute the
instruction when kernel returns the control back to the process in
userspace.
Starting from kernel v4.6 [4] there is a simple heuristic that employs a
8-bit
counter to help track if a process, after using these facilities for the
first
time, continues to use the facilities. The counters (load_fp and load_vec)
are
incremented on each context switch and if the process stops using the FP or
VEC
facilities then they are disabled again with FP/VEC/VSX save/restore on
context
switches being disabled as well in order to improve the performance on
context
switches by avoiding the FP/VEC/VEX register save/restore.
Either way (before or after the change introduced in v4.6) *that mechanism
is
opaque to userspace*, particularly to the process using these facilities.
If a
given facility is not enabled by the kernel (in case the CPU does not
support
it, kernel sends a SIGILL to the process). It's possible to inspect the
thread
member dynamics/state from userspace using tools like 'systemtap' (for
exemple, this simple script can be used to inspect a VRSAVE registers on
given
thread that is running a program called 'vrsave_' [5]) or using the 'perf'
tool.
"tsk->thread.used_vsr" [6] is actually associated to the VSX facility
whilst
MSR_VEC is associated to the VEC/VMX/Altivec facility [7], so
"tsk->thread.used_vsr" is set to 1 once a VSX instruction is used (if it's
a new
process or if the load_fp and load_vec counters overflowed and became zero
disabling VSX or if only FP or only VEC - not both - were used in the
process).
In that case kernel will also enable the VSX by MSR |= MSR_VSX. A similar
mechanism drives the FP (MSR_FP) and the VEC (MSR_VEC) facilities.
If both FP and VEC facilities are used the VSX facility is enabled
automatically
since FP+VEC regsets == VSX regset [8].
Thus as this mechanism is entirely opaque to userspace I understand that if
a
program has to tell to kernel it wants to use any of these facilities
(FP/VEC/VEC) before using it there is something wrong going in kernelspace.
Martin and Michi, if you want any help on drilling it further at kernel
side
please let me know, maybe I can help.
I didn't have the chance to reproduce the crash yet, so if I find anything
meaningful about it tomorrow I'll keep you posted.
Kind regards,
Gustavo
[1]
https://github.com/torvalds/linux/blob/master/arch/powerpc/kernel/exceptions-64s.S#L851-L869
(FP)
[2]
https://github.com/torvalds/linux/blob/master/arch/powerpc/kernel/exceptions-64s.S#L1197-L1211
(VEC/VMX/Altivec)
[3]
https://github.com/torvalds/linux/blob/master/arch/powerpc/kernel/exceptions-64s.S#L1197-L1211
(VSX)
[4]
https://github.com/torvalds/linux/commit/70fe3d980#diff-ef76830326856a12ea2b45630123d1adR239
[5] http://cr.openjdk.java.net/~gromero/script.d
[6]
https://github.com/torvalds/linux/commit/70fe3d980#diff-cc409475871baa8652ae4a5b4be7f715R310
[7]
https://github.com/torvalds/linux/commit/70fe3d980#diff-cc409475871baa8652ae4a5b4be7f715R250
[8]
https://github.com/torvalds/linux/commit/70fe3d980#diff-cc409475871baa8652ae4a5b4be7f715R437
> Best regards,
>
> Martin
>
> *From:*Michihiro Horie <HORIE at jp.ibm.com>
> *Sent:* Dienstag, 4. September 2018 07:32
> *To:* Doerr, Martin <martin.doerr at sap.com>
> *Cc:* Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Gustavo Romero
<gromero at linux.vnet.ibm.com>; hotspot compiler
<hotspot-compiler-dev at openjdk.java.net>; hotspot-dev at openjdk.java.net
> *Subject:* RE: RFR: 8208171: PPC64: Enrich SLP support
>
> Hi Goetz, Martin, and Gustavo,
>
>
>>First, this should have been reviewed on hotspot-compiler-dev. It is
clearly
>>a compiler change. _
> _>
http://mail.openjdk.java.net/mailman/listinfo
<http://mail.openjdk.java.net/mailman/listinfo> says that hotspot-dev is
for
>>"Technical discussion about the development of the HotSpot virtual
machine that's not specific to any particular component"
>>while hotspot-compiler-dev is for
>>"Technical discussion about the development of the HotSpot bytecode
compilers"
> I understood the instruction and would use hotspot-compiler-dev in future
RFRs, thanks.
>
>
>> Why do you rename vnoreg to vnoregi?
> I followed the way of coding for vsnoreg and vsnoregi, but the renaming
does not look necessary. I would get this part back. Should I also rename
vsnoregi to vsnoreg?
>
>
>>we noticed jtreg test failures when using this change:
>>compiler/runtime/safepoints/TestRegisterRestoring.java
>>compiler/runtime/Test7196199.java
>>
>>TestRegisterRestoring is a simple test which returns arbitrary results
instead of 10000.
>>
>>We didn't see it on all machines, so it might be an issue with
saving&restoring VR registers in the signal handler.
>>The machine which I have used has "SUSE Linux Enterprise Server 12 SP3"
with kernel 4.4.126-94.22-default.
> Thank you for letting me know the issue, I will try to reproduce this on
a SUSE machine.
>
>
>>I also noticed that "-XX:-SuperwordUseVSX" crashes with bad ad file when
your patch is applied. Looks like matching the vector nodes needs to be
prevented.
> Thank you for pointing out another issue. Currently I do not hit this
problem, but preventing to match the vector nodes makes sense to avoid the
crash. I did not prepare match rules for non-vector nodes, so it might be
better to prepare them similarly like the Replicate* rules, in any case.
>
>
> Gustavo, thanks for the wrap-up!
>
>
> Best regards,
> --
> Michihiro,
> IBM Research - Tokyo
>
> Inactive hide details for "Doerr, Martin" ---2018/09/04 02:18:24---Hi
Gustavo and Michihiro, we noticed jtreg test failures whe"Doerr, Martin"
---2018/09/04 02:18:24---Hi Gustavo and Michihiro, we noticed jtreg test
failures when using this change:
>
> From: "Doerr, Martin" <martin.doerr at sap.com <mailto:martin.doerr at sap.com
>>
> To: Gustavo Romero <gromero at linux.vnet.ibm.com <
mailto:gromero at linux.vnet.ibm.com>>, "Lindenmaier, Goetz"
<goetz.lindenmaier at sap.com <mailto:goetz.lindenmaier at sap.com>>, Michihiro
Horie <HORIE at jp.ibm.com <mailto:HORIE at jp.ibm.com>>
> Cc: hotspot compiler <hotspot-compiler-dev at openjdk.java.net <
mailto:hotspot-compiler-dev at openjdk.java.net>>,
"hotspot-dev at openjdk.java.net <mailto:hotspot-dev at openjdk.java.net>"
<hotspot-dev at openjdk.java.net <mailto:hotspot-dev at openjdk.java.net>>
> Date: 2018/09/04 02:18
> Subject: RE: RFR: 8208171: PPC64: Enrich SLP support
>
>
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>
>
>
> Hi Gustavo and Michihiro,
>
> we noticed jtreg test failures when using this change:
> compiler/runtime/safepoints/TestRegisterRestoring.java
> compiler/runtime/Test7196199.java
>
> TestRegisterRestoring is a simple test which returns arbitrary results
instead of 10000.
>
> We didn't see it on all machines, so it might be an issue with
saving&restoring VR registers in the signal handler.
> The machine which I have used has "SUSE Linux Enterprise Server 12 SP3"
with kernel 4.4.126-94.22-default.
>
> That's what I found out so far. Maybe you have an idea?
>
> I also noticed that "-XX:-SuperwordUseVSX" crashes with bad ad file when
your patch is applied. Looks like matching the vector nodes needs to be
prevented.
>
> Best regards,
> Martin
>
>
> -----Original Message-----
> From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net <
mailto:hotspot-dev-bounces at openjdk.java.net>> On Behalf Of Gustavo Romero
> Sent: Montag, 3. September 2018 14:57
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com <
mailto:goetz.lindenmaier at sap.com>>; Michihiro Horie <HORIE at jp.ibm.com <
mailto:HORIE at jp.ibm.com>>
> Cc: hotspot compiler <hotspot-compiler-dev at openjdk.java.net <
mailto:hotspot-compiler-dev at openjdk.java.net>>;
hotspot-dev at openjdk.java.net <mailto:hotspot-dev at openjdk.java.net>
> Subject: Re: RFR: 8208171: PPC64: Enrich SLP support
>
> Hi Goetz,
>
> On 09/03/2018 09:27 AM, Lindenmaier, Goetz wrote:
>> Also, I can not find all of the mail traffic in
>>
http://mail.openjdk.java.net/pipermail/hotspot-dev/2018-August/thread.html.
>> Is this a problem of the pipermail server?
>>
>> For some reason this webrev lacks the links to browse the diffs.
>> Do you need to use a more recent webrev? You can obtain it with
>> hg clone http://hg.openjdk.java.net/code-tools/webrev/ .
>
> Yes, probably it was a problem of the pipermail or in some relay.
> I noted the same thing, i.e. at least one Michi reply arrived
> to me but missed a ML.
>
> The initial discussion is here:
>
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2018-July/003613.html
>
> I understand Martin reviewed the last webrev in that thread, which is
> http://cr.openjdk.java.net/~mhorie/8208171/webrev.01/ <
http://cr.openjdk.java.net/%7Emhorie/8208171/webrev.01/> (taken from
>
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2018-July/003615.html
)
>
> Martin's review of webrev.01:
>
http://mail.openjdk.java.net/pipermail/hotspot-dev/2018-August/033958.html
>
> and Michi's reply to Martin's review of webrev.01:
>
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2018-August/003632.html
(with webrev.02,
> taken from
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2018-August/003632.html
).
>
> and your last review:
>
http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2018-September/030419.html
>
>
> HTH.
>
> Best regards,
> Gustavo
>
>> Why do you rename vnoreg to vnoregi?
>>
>> Besides that the change is fine, thanks for implementing this!
>>
>> Best regards,
>> Goetz.
>>
>>
>>> -----Original Message-----
>>> From: Doerr, Martin
>>> Sent: Dienstag, 28. August 2018 19:35
>>> To: Gustavo Romero <gromero at linux.vnet.ibm.com <
mailto:gromero at linux.vnet.ibm.com>>; Michihiro Horie
>>> <HORIE at jp.ibm.com <mailto:HORIE at jp.ibm.com>>
>>> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com <
mailto:goetz.lindenmaier at sap.com>>; hotspot-
>>> dev at openjdk.java.net <mailto:dev at openjdk.java.net>;
ppc-aix-port-dev at openjdk.java.net <mailto:ppc-aix-port-dev at openjdk.java.net
>; Simonis, Volker
>>> <volker.simonis at sap.com <mailto:volker.simonis at sap.com>>
>>> Subject: RE: RFR: 8208171: PPC64: Enrich SLP support
>>>
>>> Hi Michihiro,
>>>
>>> thank you for implementing it. I have just taken a first look at your
>>> webrev.01.
>>>
>>> It looks basically good. Only the Power version check seems to be
incorrect.
>>> VM_Version::has_popcntb() checks for Power5.
>>> I believe most instructions are available with Power7.
>>> Some ones (vsubudm, ..., vmmuluwm, vpopcntw) were introduced with
>>> Power8?
>>> We should check this carefully.
>>>
>>> Also, indentation in register_ppc.hpp could get improved.
>>>
>>> Thanks and best regard,
>>> Martin
>>>
>>>
>>> -----Original Message-----
>>> From: Gustavo Romero <gromero at linux.vnet.ibm.com <
mailto:gromero at linux.vnet.ibm.com>>
>>> Sent: Donnerstag, 26. Juli 2018 16:02
>>> To: Michihiro Horie <HORIE at jp.ibm.com <mailto:HORIE at jp.ibm.com>>
>>> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com <
mailto:goetz.lindenmaier at sap.com>>; hotspot-
>>> dev at openjdk.java.net <mailto:dev at openjdk.java.net>; Doerr, Martin
<martin.doerr at sap.com <mailto:martin.doerr at sap.com>>; ppc-aix-
>>> port-dev at openjdk.java.net <mailto:port-dev at openjdk.java.net>; Simonis,
Volker <volker.simonis at sap.com <mailto:volker.simonis at sap.com>>
>>> Subject: Re: RFR: 8208171: PPC64: Enrich SLP support
>>>
>>> Hi Michi,
>>>
>>> On 07/26/2018 01:43 AM, Michihiro Horie wrote:
>>>> I updated webrev:
>>>> http://cr.openjdk.java.net/~mhorie/8208171/webrev.01/ <
http://cr.openjdk.java.net/%7Emhorie/8208171/webrev.01/>
>>>
>>> Thanks for providing an updated webrev and for fixing indentation and
>>> function
>>> order in assembler_ppc.inline.hpp as well. I have no further
comments :)
>>>
>>>
>>> Best Regards,
>>> Gustavo
>>>
>>>>
>>>> Best regards,
>>>> --
>>>> Michihiro,
>>>> IBM Research - Tokyo
>>>>
>>>> Inactive hide details for Gustavo Romero ---2018/07/25 23:05:32---Hi
Michi,
>>> On 07/25/2018 02:43 AM, Michihiro Horie wrote:Gustavo Romero ---
>>> 2018/07/25 23:05:32---Hi Michi, On 07/25/2018 02:43 AM, Michihiro Horie
>>> wrote:
>>>>
>>>> From: Gustavo Romero <gromero at linux.vnet.ibm.com <
mailto:gromero at linux.vnet.ibm.com>>
>>>> To: Michihiro Horie/Japan/IBM at IBMJP, ppc-aix-port-
>>> dev at openjdk.java.net <mailto:dev at openjdk.java.net>,
hotspot-dev at openjdk.java.net <mailto:hotspot-dev at openjdk.java.net>
>>>> Cc: goetz.lindenmaier at sap.com <mailto:goetz.lindenmaier at sap.com>,
volker.simonis at sap.com <mailto:volker.simonis at sap.com>, "Doerr, Martin"
>>> <martin.doerr at sap.com <mailto:martin.doerr at sap.com>>
>>>> Date: 2018/07/25 23:05
>>>> Subject: Re: RFR: 8208171: PPC64: Enrich SLP support
>>>>
>>>>
-------------------------------------------------------------------------------------------
>>>
----------------------------------------------------------------------------------------------
>>>
----------------------------------------------------------------------------------------------
>>>
----------------------------------------------------------------------------------------------
>>>
----------------------------------------------------------------------------------------------
>>>
----------------------------------------------------------------------------------------------
>>>
----------------------------------------------------------------------------------------------
>>>
----------------------------------------------------------------------------------------------
>>>
----------------------------------------------------------------------------------------------
>>>
----------------------------------------------------------------------------------------------
>>> -----------------------------------------------------
>>>>
>>>>
>>>>
>>>> Hi Michi,
>>>>
>>>> On 07/25/2018 02:43 AM, Michihiro Horie wrote:
>>>> > Dear all,
>>>> >
>>>> > Would you review the following change?
>>>> > Bug: https://bugs.openjdk.java.net/browse/JDK-8208171
>>>> > Webrev: http://cr.openjdk.java.net/~mhorie/8208171/webrev.00 <
http://cr.openjdk.java.net/%7Emhorie/8208171/webrev.00>
>>>> >
>>>> > This change adds support for vectorized arithmetic calculation
with SLP.
>>>> >
>>>> > The to_vr function is added to convert VSR to VR. Currently, vecX
is
>>> associated with a VSR class vs_reg that only defines VSR32-51 in
ppc.ad,
>>> which are exactly overlapped with VRs. Instruction APIs receiving VRs
use the
>>> to_vr via vecX. Another thing is the change in sqrtF_reg to enable the
>>> matching with SqrtVF. I think the change in sqrtF_reg would be fine due
to
>>> the ConvD2FNode::Value in convertnode.cpp.
>>>>
>>>> Looks good. Just a few comments:
>>>>
>>>> - In vmul4F_reg() would it be reasonable to use xvmulsp instead of
>>> vmaddfp in
>>>> order to avoid the splat?
>>>>
>>>> - Although all instructions added by your change where introduced in
ISA
>>> 2.06,
>>>> so POWER7 and above are OK, as I see probes for
>>> PowerArchictecturePPC64=6|5 in
>>>> vm_version_ppc.cpp (line 64), I'm wondering if there is any
control point
>>> to
>>>> guarantee that these instructions won't be emitted on a CPU that
does
>>> not
>>>> support them.
>>>>
>>>> - I think that in general string in format %{} are in upper case. For
instance,
>>>> this the current output on optoassembly for vmul4F:
>>>>
>>>> 2941835 5b4 ADDI R24, R24, #64
>>>> 2941836 5b8 vmaddfp VSR32,VSR32,VSR36 ! mul packed4F
>>>> 2941837 5c0 STXVD2X [R17], VSR32 // store 16-byte
Vector
>>>>
>>>> I think it would be better to be in upper case instead. I also
think that if
>>>> the node match emits more than one instruction all instructions
must be
>>> listed
>>>> in format %{}, since it's meant for detailed debugging. Finally I
think it
>>>> would be better to replace \t! by \t// in that string (unless I'm
missing any
>>>> special meaning for that char). So for vmul4F it would be
something like:
>>>>
>>>> 2941835 5b4 ADDI R24, R24, #64
>>>> VSPLTISW VSR34, 0 // Splat 0 imm in
VSR34
>>>> 2941836 5b8 VMADDFP VSR32,VSR32,VSR36,VSR34 // Mul packed4F
>>>> 2941837 5c0 STXVD2X [R17], VSR32 // store 16-byte
Vector
>>>>
>>>>
>>>> But feel free to change anything just after you get additional
reviews :)
>>>>
>>>>
>>>> > I confirmed this change with JTREG. In addition, I used attached
micro
>>> benchmarks.
>>>> > /(See attached file: slp_microbench.zip)/
>>>>
>>>> Thanks for sharing it.
>>>> Btw, another option to host it would be in the CR
>>>> server, in http://cr.openjdk.java.net/~mhorie/8208171 <
http://cr.openjdk.java.net/%7Emhorie/8208171>
>>>>
>>>>
>>>> Best regards,
>>>> Gustavo
>>>>
>>>> >
>>>> > Best regards,
>>>> > --
>>>> > Michihiro,
>>>> > IBM Research - Tokyo
>>>> >
>>>>
>>>>
>>>>
>>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20180905/19f780c9/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: graycol.gif
Type: image/gif
Size: 105 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20180905/19f780c9/graycol-0001.gif>
More information about the hotspot-compiler-dev
mailing list