Fine with me.

-phil.

On 2/19/19, 4:10 AM, Doerr, Martin wrote:

Hi Matthias,

build has passed with this version. I think you can ship it.

Best regards,

Martin

*From:*Baesken, Matthias
*Sent:* Dienstag, 19. Februar 2019 09:57
*To:* Philip Race <philip.r...@oracle.com>
*Cc:* 2d-dev@openjdk.java.net; Doerr, Martin <martin.do...@sap.com>
*Subject:* RE: [OpenJDK 2D-Dev] FW: RFR : 8218965: aix: support xlclang++ in the compiler detection

Hello, for some reason a “c” (__ibmxlc__) was in my last webrev that does not belong there; the macro name is __ibmxl__ :

118 #elif !defined(HB_NO_MT) && defined(_AIX) && (defined(__IBMCPP__) || defined(__ibmxl__))

New webrev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8218965.2/ <http://cr.openjdk.java.net/%7Embaesken/webrevs/8218965.2/>

>

>If there's a hb release with it soon I can use that or I will try
>to remember to re-apply it ...

>

Thanks for looking into it  (and for trying to remember 😉  ) !

Best regards, Matthias

*From:*Philip Race <philip.r...@oracle.com <mailto:philip.r...@oracle.com>>
*Sent:* Dienstag, 19. Februar 2019 01:18
*To:* Baesken, Matthias <matthias.baes...@sap.com <mailto:matthias.baes...@sap.com>> *Cc:* 2d-dev@openjdk.java.net <mailto:2d-dev@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com <mailto:martin.do...@sap.com>> *Subject:* Re: [OpenJDK 2D-Dev] FW: RFR : 8218965: aix: support xlclang++ in the compiler detection

Yes .. I see that ...
https://github.com/harfbuzz/harfbuzz/commit/5c2bb1de8de31fecf0dae2ef905b896e42d39f1d

.. looks ok ..
although I soon need to upgrade harfbuzz in JDK and the
current release of 2.3.1 doesn't have that change.

If there's a hb release with it soon I can use that or I will try
to remember to re-apply it ...

-phil.

On 2/18/19, 7:08 AM, Baesken, Matthias wrote:

    FYI -    this change

    http://cr.openjdk.java.net/~mbaesken/webrevs/8218965.1/
    <http://cr.openjdk.java.net/%7Embaesken/webrevs/8218965.1/>

includes a small harfbuzz fix by Martin as well ; he fixed it as well in the upstream harfbuzz project .

    Best regards, Matthias

    *From:*Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>
    <mailto:magnus.ihse.bur...@oracle.com>
    *Sent:* Montag, 18. Februar 2019 15:46
    *To:* Doerr, Martin <martin.do...@sap.com>
    <mailto:martin.do...@sap.com>
    *Cc:* Baesken, Matthias <matthias.baes...@sap.com>
    <mailto:matthias.baes...@sap.com>; build-...@openjdk.java.net
    <mailto:build-...@openjdk.java.net>
    *Subject:* Re: RFR : 8218965: aix: support xlclang++ in the
    compiler detection

    Looks good to me.

    /Magnus


    18 feb. 2019 kl. 15:37 skrev Doerr, Martin <martin.do...@sap.com
    <mailto:martin.do...@sap.com>>:

        Hi Matthias,

        excellent. Looks good to me. This should make AIX ready for
        JEP 347.

        Thanks

        Martin

        *From:*Baesken, Matthias
        *Sent:* Montag, 18. Februar 2019 13:53
        *To:* Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com
        <mailto:magnus.ihse.bur...@oracle.com>>;
        'build-...@openjdk.java.net
        <mailto:build-...@openjdk.java.net>'
        <build-...@openjdk.java.net <mailto:build-...@openjdk.java.net>>
        *Cc:* Doerr, Martin <martin.do...@sap.com
        <mailto:martin.do...@sap.com>>
        *Subject:* RE: RFR : 8218965: aix: support xlclang++ in the
        compiler detection

        Hello Martin and Magnus,

        I  included Martin’s  harfbuzz fix  and adjusted  the  xlc
        version check   ( renamed  variable to XLC_USES_CLANG and also
        check the *TOOLCHAIN_PATH ) .*

        >

        >If we're talking about a short migration story, where soon XLC
        16 will be required, and we can just replace

        >TOOLCHAIN_CC_BINARY_xlc="xlc_r"

        >with

        >TOOLCHAIN_CC_BINARY_xlc="xlclang"

        > then I can accept it anyway, so we don't need to complicate
        things.
        >

        Yes , that’s the idea -  to do the replacement above   sooner
        or later ;  depends of course also on the  introduction of the
        C++11/14 features in the code base .

        New webrev :

        http://cr.openjdk.java.net/~mbaesken/webrevs/8218965.1/
        <http://cr.openjdk.java.net/%7Embaesken/webrevs/8218965.1/>

        Best Regards, Matthias

        *From:*Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com
        <mailto:magnus.ihse.bur...@oracle.com>>
        *Sent:* Montag, 18. Februar 2019 11:18
        *To:* Baesken, Matthias <matthias.baes...@sap.com
        <mailto:matthias.baes...@sap.com>>;
        'build-...@openjdk.java.net
        <mailto:build-...@openjdk.java.net>'
        <build-...@openjdk.java.net <mailto:build-...@openjdk.java.net>>
        *Cc:* Doerr, Martin <martin.do...@sap.com
        <mailto:martin.do...@sap.com>>
        *Subject:* Re: RFR : 8218965: aix: support xlclang++ in the
        compiler detection

        On 2019-02-15 14:30, Baesken, Matthias wrote:

                Are they both pointing to the same binary, and the mode of 
operation

                (legacy xlc vs xlclang) is determined by the name of the 
executable?

            Hello, in the installation I use   I have separate  binaries .

                Is xlclang++ always available for version 16+ of xlc?

            I think so,  as least I am not  aware of an installation mode with 
separate  binaries .

            However I am not an XLC  installation guru😊  .

                If so, maybe we should just make sure we call the compiler with 
the

                correct name if version 16+ is detected?

            I thought  that we currently  first set  the  toolchain  name and 
then  set a fix  name for the binary  and check the version .

            But I might be wrong.  Maybe  we need  to adjust this .

            Or just at some future  point in time  declare  xlc16   as minimum  
 requirement  (this makes things  easier , we can then use  the new binary 
names   ).


        Yeah, we can adjust the process if needed. And to solve this
        *properly*, we should. I still think this looks like the wrong
        way to do it. But...

        If we're talking about a short migration story, where soon XLC
        16 will be required, and we can just replace

        TOOLCHAIN_CC_BINARY_xlc="xlc_r"

        with

        TOOLCHAIN_CC_BINARY_xlc="xlclang"

        then I can accept it anyway, so we don't need to complicate
        things.

        I also don't like how xlclang is just run from the path, but
        OTOH it's only you guys who are going to run that in practice,
        and it's just going to be temporary, so, whatever.

        The name AIX_USE_CLANG is not really correct, though. This is
        not about AIX. It should be XLC_USE_CLANG (or maybe better
        XLC_USES_CLANG, even perhaps XLC_IS_CLANG?!). And, as I said,
        it should use true/false, not 0/1.

        If you fix this, and we agree that this is a temporary
        measure, I'm OK with the patch.

        /Magnus



Reply via email to