hotspot: problem with bitwise rotation on PowerLE
Andrew Hughes
gnu.andrew at redhat.com
Wed May 20 14:56:44 UTC 2015
----- Original Message -----
> I have now also fixed this issue in our ppc-aix-port/jdk7u repository
>
> http://hg.openjdk.java.net/ppc-aix-port/jdk7u/hotspot/rev/c1394d3bdb6a
>
> Please consider pulling this change into your local repositories if
> you are building on Linux/ppc64 or AIX because it will not be
> integrated into the main jdk7u/hotspot repository (simply because the
> main jdk7u/hotspot does not yet contain the ppc64 port).
>
I plan to do a sync with the ppc-aix-port/jdk7u tree before our next
releases of IcedTea (2.6.0 ASAP and 2.5.6 for the July CPU).
> Regards,
> Volker
>
>
> On Wed, May 20, 2015 at 12:37 PM, Tony Reix <tony.reix at atos.net> wrote:
> > What about jdk7 ?
> >
> >
> > Cordialement,
> >
> > Tony Reix
> >
> > Bull - ATOS
> > IBM Coop Architect & Technical Leader
> > Office : +33 (0) 4 76 29 72 67
> > 1 rue de Provence - 38432 Échirolles - France
> > www.atos.net
> >
> > ________________________________________
> > De : ppc-aix-port-dev [ppc-aix-port-dev-bounces at openjdk.java.net] de la
> > part de Volker Simonis [volker.simonis at gmail.com]
> > Envoyé : mercredi 20 mai 2015 10:45
> > À : Alexander Smundak
> > Cc : ppc-aix-port-dev at openjdk.java.net
> > Objet : Re: hotspot: problem with bitwise rotation on PowerLE
> >
> > I just wanted to let you know that this issue has now been fixed in
> > jdk9 and jdk8u.
> >
> > Currently the change for 9 is in the jdk9/hs-comp/hotspot team
> > repository but it should propagate to jdk9/dev/hotspot soon (in a week
> > or two).
> >
> > The fix for 8u is currently in jdk8u/hs-dev/hotspot but should
> > propagate to jdk8u/dev in time for the upcoming 8u60 release.
> >
> > You can watch the status of the fix at:
> >
> > https://bugs.openjdk.java.net/browse/JDK-8080190 for jdk9 and:
> > https://bugs.openjdk.java.net/browse/JDK-8080749 for the 8u downport.
> >
> > Thanks for reporting this.
> > Volker
> >
> >
> > On Wed, May 13, 2015 at 11:36 AM, Volker Simonis
> > <volker.simonis at gmail.com> wrote:
> >> Thanks, but I've already written a minimal test case myself.
> >>
> >> I just asked for including the initial test by Jan because it also
> >> exercises some other interesting parts of the optimizer and may thus
> >> be a valuable test case in itself.
> >>
> >> Regards,
> >> Volker
> >>
> >>
> >> On Wed, May 13, 2015 at 12:34 AM, Alexander Smundak <asmundak at google.com>
> >> wrote:
> >>> I have somewhat more isolated test (just calling
> >>> rotateRigth/rotateLeft methods from my previous e-mail with constant
> >>> distance argument that reveals the problem. Please let me know if you
> >>> are interested.
> >>>
> >>> On Tue, May 12, 2015 at 2:17 PM, Jan S Rellermeyer
> >>> <rellermeyer at us.ibm.com> wrote:
> >>>> I have also been able test the patch in our setup and it works. Thanks
> >>>> for
> >>>> figuring this out.
> >>>>
> >>>> Do you happen to know when the next 8 update release is scheduled that
> >>>> would
> >>>> contain the fix?
> >>>>
> >>>> Regarding coverage by the IBM OCA and the ability to contribute the hash
> >>>> code for regression testing, I am checking this right now with the
> >>>> people
> >>>> who passed the code on to me.
> >>>>
> >>>> --Jan.
> >>>>
> >>>> Alexander Smundak ---05/12/2015 03:59:48 PM---I can confirm that the
> >>>> patch
> >>>> works. I tried to write the test that covers both 'rotlI_reg_immi8' and
> >>>>
> >>>> From: Alexander Smundak <asmundak at google.com>
> >>>> To: Volker Simonis <volker.simonis at gmail.com>
> >>>> Cc: Jan S Rellermeyer/Austin/IBM at IBMUS,
> >>>> "ppc-aix-port-dev at openjdk.java.net"
> >>>> <ppc-aix-port-dev at openjdk.java.net>
> >>>> Date: 05/12/2015 03:59 PM
> >>>> Subject: Re: hotspot: problem with bitwise rotation on PowerLE
> >>>>
> >>>> ________________________________
> >>>>
> >>>>
> >>>>
> >>>> I can confirm that the patch works. I tried to write the test that
> >>>> covers
> >>>> both
> >>>> 'rotlI_reg_immi8' and ' rotrI_reg_immi8' patterns but I didn't manage
> >>>> to match the latter,
> >>>> somehow all of the following methods inline to ROTLWI:
> >>>>
> >>>> static int rotateRight1(int i, int distance) { return (i >>>
> >>>> distance) | (i << -distance); }
> >>>> static int rotateRight2(int i, int distance) { return (i <<
> >>>> -distance) | (i >>> distance); }
> >>>>
> >>>> These two inline to
> >>>> rotlwi Rx, Ry, (32-distance)
> >>>>
> >>>> static int rotateLeft1(int i, int distance) { return (i << distance)
> >>>> | (i >>> -distance); }
> >>>> static int rotateLeft2(int i, int distance) { return (i >>>
> >>>> -distance) | (i << distance); }
> >>>> And these to inline to
> >>>> rotlwi Rx, Ry, distance
> >>>>
> >>>> How can such a bug escape the hotspot test? I don't have access to
> >>>> JCK, but isn't it supposed to cover this?
> >>>>
> >>>> On Tue, May 12, 2015 at 11:31 AM, Volker Simonis
> >>>> <volker.simonis at gmail.com> wrote:
> >>>>> Hi,
> >>>>>
> >>>>> I think I've finally understood and fixed this issue.
> >>>>>
> >>>>> The problem was that we initially took the "rotate left/right by 8-bit
> >>>>> immediate" instructions from x86_64. But on x86_64 the rotate
> >>>>> instructions for 32-bit integers do in fact really take 8-bit
> >>>>> immediates (seems like the Intel guys had to many spare bits when they
> >>>>> designed their instruction set :) On the other hand, the rotate
> >>>>> instructions on Power only use 5 bit to encode the rotation distance.
> >>>>>
> >>>>> We do check for this limit, but only in the debug build. That's why
> >>>>> you saw the assertion in the fastdebug build.
> >>>>>
> >>>>> I think the right solution for this problem is to simply mask the
> >>>>> rotation distance with 0x1f (which is equivalent to taking the
> >>>>> distance modulo 32).
> >>>>>
> >>>>> Can you please verify that the proposed fix also works for you on LE?
> >>>>>
> >>>>> https://bugs.openjdk.java.net/browse/JDK-8080190
> >>>>> http://cr.openjdk.java.net/~simonis/webrevs/2015/8080190/
> >>>>>
> >>>>> If yes I can send it out for review and push it to our jdk9 and jdk8u
> >>>>> repositories.
> >>>>>
> >>>>> @Jan: have you signed the Oracle Contributor Agreement or are you
> >>>>> covered by the company-wide IBM OCA? I'd like to take your test as a
> >>>>> regresstion test into the OpenJDK HotSpot repository, but this is only
> >>>>> possible if you're a Contributor[2].
> >>>>>
> >>>>> Thank you and best regards,
> >>>>> Volker
> >>>>>
> >>>>> PS: the issue didn't appear in our internal SAP JVM version because we
> >>>>> already mask the inputs of the corresponding shift nodes in the ideal
> >>>>> graph but for some unknown reasons this shared changes haven’t been
> >>>>> contributed along with our ppc port so far.
> >>>>>
> >>>>> [1] http://www.oracle.com/technetwork/community/oca-486395.html
> >>>>> [2] http://openjdk.java.net/bylaws#contributor
> >>>>>
> >>>>>
> >>>>> On Tue, May 12, 2015 at 2:38 PM, Volker Simonis
> >>>>> <volker.simonis at gmail.com> wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> this problem is not LE-related. I could reproduce it on BE as well. I
> >>>>>> also don't think that the fix proposed by Jan is completely right. I
> >>>>>> think it only helps because it leads to another instruction being
> >>>>>> matched. The problem is that the URshift has a negative input which
> >>>>>> should not be. It should have been converted to it's positive
> >>>>>> equivalent modulo 32 (as described in the JLS 15.19 -
> >>>>>> https://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.19).
> >>>>>>
> >>>>>> Strange enough, in our internal version, the error doesn't occur so I
> >>>>>> should actually easy find the difference. Unfortunately, I didn't saw
> >>>>>> it until now :(
> >>>>>> I'll keep you posted once I found out more...
> >>>>>>
> >>>>>> Regards,
> >>>>>> Volker
> >>>>>>
> >>>>>> On Tue, May 12, 2015 at 6:06 AM, Alexander Smundak
> >>>>>> <asmundak at google.com>
> >>>>>> wrote:
> >>>>>>> Looking into this. Sorry for the late response, we were moved into a
> >>>>>>> different office over the weekend.
> >>>>>>>
> >>>>>>> On Fri, May 8, 2015 at 1:01 PM, Jan S Rellermeyer
> >>>>>>> <rellermeyer at us.ibm.com> wrote:
> >>>>>>>> I am helping people in IBM who have encountered a hotspot bug that
> >>>>>>>> surfaces
> >>>>>>>> when using bitwise rotations (such as through Integer.rotateRight).
> >>>>>>>> I
> >>>>>>>> have
> >>>>>>>> attached a small test program that illustrates the problem. When
> >>>>>>>> running
> >>>>>>>> this on Power LE (I don't think that anybody tried on BE, though)
> >>>>>>>> the
> >>>>>>>> results of the hash function are incorrect.
> >>>>>>>>
> >>>>>>>> The bug goes away when disabling inlining. After digging through the
> >>>>>>>> generated code I found out that it contained some suspicious fnmadd
> >>>>>>>> instructions where you would rather expect some kind of left rotate.
> >>>>>>>> I
> >>>>>>>> could
> >>>>>>>> track down that the bug was caused by the immediate value for the
> >>>>>>>> right
> >>>>>>>> shift constant of what should be a rlwinm node to be negative and
> >>>>>>>> thereby
> >>>>>>>> bleeding into the opcode (which unfortunately still created a valid
> >>>>>>>> Power
> >>>>>>>> instruction, namely the fnmadd). Running with a fastdebug build
> >>>>>>>> causes
> >>>>>>>> an
> >>>>>>>> assertion to fail.
> >>>>>>>>
> >>>>>>>> The attached patch does fix the problem and so does an alternative
> >>>>>>>> patch
> >>>>>>>> that replaces the URshift with a UShift but I am not entirely sure
> >>>>>>>> if
> >>>>>>>> this
> >>>>>>>> really fixes the root cause or rather just prevents the rotlI
> >>>>>>>> instruction
> >>>>>>>> from being matched by the ReduceInst.
> >>>>>>>>
> >>>>>>>> --Jan.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Dr. Jan S. Rellermeyer
> >>>>>>>> IBM Research
> >>>>>>>> RSM - Austin Research Lab
> >>>>>>>> rellermeyer_at_us.ibm.com
> >>>>>>>> The University of Texas at Austin
> >>>>>>>> Adjunct Assistant Professor
> >>>>>>>> jrellerm_at_cs.utexas.edu
> >>>>>>>>
> >>>>>>>> (See attached file: hash.java)(See attached file: rotl.patch)
> >>>>
> >>>>
> >>>>
>
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
PGP Key: ed25519/35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04 C5A0 CFDA 0F9B 3596 4222
PGP Key: rsa4096/248BDC07 (hkp://keys.gnupg.net)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B DC07
More information about the ppc-aix-port-dev
mailing list