[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] RFR : 8193515 : AIX : new Harfbuzz 1.7.1 version fails to compile
From: Volker Simonis <volker.simonis () gmail ! com>
Date: 2017-12-21 14:05:50
Message-ID: CA+3eh12r7Oe-1+SDw4YypQnnNj_X5K1g+40HT0zR=p69AErriA () mail ! gmail ! com
[Download RAW message or body]
FYI: my patch has been accepted and pushed upstream into Harfbuzz:
Closed #655 https://github.com/harfbuzz/harfbuzz/issues/655 via #656
https://github.com/harfbuzz/harfbuzz/pull/656
On Tue, Dec 19, 2017 at 11:43 AM, Volker Simonis
<volker.simonis@gmail.com> wrote:
> Thanks Phil!
>
> I've just pushed this to jdk/jdk10.
>
> I've also opened an issue [1] in the upstream Harfbuzz project and
> submitted a pull request with the fix [2].
>
> So hopefully this won't be an issue with the next integration any more.
>
> Regards,
> Volker
>
> [1] https://github.com/harfbuzz/harfbuzz/issues/655
> [2] https://github.com/harfbuzz/harfbuzz/pull/656
>
> On Mon, Dec 18, 2017 at 6:24 PM, Philip Race <philip.race@oracle.com> wrote:
>> +1 from me ..
>>
>> yes push to jdk/jdk10.
>>
>> -phil.
>>
>>
>> On 12/18/17, 1:51 AM, Volker Simonis wrote:
>>>
>>> Hi Matthias,
>>>
>>> the change looks good and I can sponsor it. I'd just propose to
>>> slightly change the comment to "..by the overloaded versions of 'cmp'
>>> in IntType" if you don't mind. There's no need for a new webrew tough
>>> - I can make that change before pushing.
>>>
>>> I'm just waiting for one more review from the 2D group. Afterwards
>>> I'll push directly to jdk/jdk10. From my understanding, the fix will
>>> than be automatically forward-integrated into jdk/jdk.
>>>
>>> Regards,
>>> Volker
>>>
>>>
>>> On Fri, Dec 15, 2017 at 2:24 PM, Baesken, Matthias
>>> <matthias.baesken@sap.com> wrote:
>>>>
>>>> Hello, I created a second webrev :
>>>>
>>>>
>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8193515.1/
>>>>
>>>>
>>>>>> Whatever we do, can it be wrapped in an appropriate #ifdef AIX so that
>>>>>> other platforms are guaranteed to be unaffected ?
>>>>
>>>> The new webrev uses the simpler cast-version proposed by Volki , and
>>>> guards the AIX change by ifdef (suggested by Phil).
>>>>
>>>>>>> In the meantime we try to find out how latest xlC version 13 behaves .
>>>>
>>>> In the meantime I checked with XLC13 as well , but we see the error
>>>> with XLC 13 too (same as XLC 12).
>>>>
>>>> Please review the change .
>>>>
>>>> It should go later into jdk10 as well (because jdk10 contains the new
>>>> Harfbuzz 1.7.1 too ).
>>>>
>>>>
>>>> Thanks, Matthias
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Baesken, Matthias
>>>>> Sent: Donnerstag, 14. Dezember 2017 18:03
>>>>> To: 'Phil Race'<philip.race@oracle.com>; Volker Simonis
>>>>> <volker.simonis@gmail.com>
>>>>> Cc: Simonis, Volker<volker.simonis@sap.com>; 2d-dev@openjdk.java.net
>>>>> Subject: RE: [OpenJDK 2D-Dev] RFR : 8193515 : AIX : new Harfbuzz 1.7.1
>>>>> version fails to compile
>>>>>
>>>>> Hi Phil, hi Volki,
>>>>> I think wrapping Volkis version into #ifdef _AIX
>>>>> plus adding a small comment sounds like a good idea .
>>>>>
>>>>> In the meantime we try to find out how latest xlC version 13 behaves .
>>>>>
>>>>> Best regards, Matthias
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Phil Race [mailto:philip.race@oracle.com]
>>>>>> Sent: Donnerstag, 14. Dezember 2017 17:53
>>>>>> To: Volker Simonis<volker.simonis@gmail.com>; Baesken, Matthias
>>>>>> <matthias.baesken@sap.com>
>>>>>> Cc: Simonis, Volker<volker.simonis@sap.com>; 2d-dev@openjdk.java.net
>>>>>> Subject: Re: [OpenJDK 2D-Dev] RFR : 8193515 : AIX : new Harfbuzz 1.7.1
>>>>>> version fails to compile
>>>>>>
>>>>>> Whatever we do, can it be wrapped in an appropriate #ifdef AIX so that
>>>>>> other platforms are guaranteed to be unaffected ?
>>>>>>
>>>>>> For upstream you can report it at github
>>>>>> https://github.com/harfbuzz/harfbuzz
>>>>>> and see how Behdad would like to handle it.
>>>>>>
>>>>>> I expect he would want it removed once the compiler bug is fixed.
>>>>>>
>>>>>> -pgil.
>>>>>>
>>>>>> On 12/14/2017 08:13 AM, Volker Simonis wrote:
>>>>>>>
>>>>>>> Hi Matthias,
>>>>>>>
>>>>>>> thanks for addressing this issue!
>>>>>>>
>>>>>>> I'm pretty sure that his is a compiler bug and I have a short
>>>>>>> reproducer which I'll send to IBM.
>>>>>>>
>>>>>>> The problem is that xlC can't distinguish a static member function
>>>>>>> (which uses an ordinary function call) from a non-static member
>>>>>>> function (which uses a member function call) with the same name.
>>>>>>>
>>>>>>> I've just found a slightly more elegant (and less intrusive) fix. We
>>>>>>> can help the compiler to find the correct method by simply casting it
>>>>>>> to the correct version:
>>>>>>>
>>>>>>> diff -r be065f758154
>>>>>>> src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot-shape-
>>>>>>
>>>>>> complex-arabic-fallback.hh
>>>>>>>
>>>>>>> --- a/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot-
>>>>>>
>>>>>> shape-complex-arabic-fallback.hh
>>>>>>>
>>>>>>> Thu Dec 14 12:49:47 2017 +0530
>>>>>>> +++ b/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot-
>>>>>>
>>>>>> shape-complex-arabic-fallback.hh
>>>>>>>
>>>>>>> Thu Dec 14 17:11:49 2017 +0100
>>>>>>> @@ -77,7 +77,7 @@
>>>>>>>
>>>>>>> /* Bubble-sort or something equally good!
>>>>>>> * May not be good-enough for presidential candidate interviews,
>>>>>>> but good-enough for us... */
>>>>>>> - hb_stable_sort (&glyphs[0], num_glyphs, OT::GlyphID::cmp,
>>>>>>
>>>>>> &substitutes[0]);
>>>>>>>
>>>>>>> + hb_stable_sort (&glyphs[0], num_glyphs, (int(*)(const OT::GlyphID
>>>>>>> *, const OT::GlyphID *)) OT::GlyphID::cmp,&substitutes[0]);
>>>>>>>
>>>>>>> OT::Supplier<OT::GlyphID> glyphs_supplier (glyphs,
>>>>>>> num_glyphs);
>>>>>>> OT::Supplier<OT::GlyphID> substitutes_supplier (substitutes,
>>>>>>
>>>>>> num_glyphs);
>>>>>>>
>>>>>>> @@ -126,7 +126,7 @@
>>>>>>> first_glyphs_indirection[num_first_glyphs] = first_glyph_idx;
>>>>>>> num_first_glyphs++;
>>>>>>> }
>>>>>>> - hb_stable_sort (&first_glyphs[0], num_first_glyphs,
>>>>>>> OT::GlyphID::cmp,&first_glyphs_indirection[0]);
>>>>>>>
>>>>>>> + hb_stable_sort (&first_glyphs[0], num_first_glyphs, (int(*)(const
>>>>>>> OT::GlyphID *, const OT::GlyphID *)) OT::GlyphID::cmp,
>>>>>>> &first_glyphs_indirection[0]);
>>>>>>>
>>>>>>> /* Now that the first-glyphs are sorted, walk again, populate
>>>>>>> ligatures.
>>>>>
>>>>> */
>>>>>>>
>>>>>>> for (unsigned int i = 0; i< num_first_glyphs; i++)
>>>>>>>
>>>>>>> I'll also try to bring this change upstream into Harfbuzz, but we need
>>>>>>> to push this into the new jdk/jdk10 repo because there will be no more
>>>>>>> HarfBuzz integration for Java 10.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Volker
>>>>>>>
>>>>>>>
>>>>>>> On Thu, Dec 14, 2017 at 4:10 PM, Baesken, Matthias
>>>>>>> <matthias.baesken@sap.com> wrote:
>>>>>>>>
>>>>>>>> Hello, after upgrading to new Harfbuzz 1.7.1 the openjdk build
>>>>>>>> fails on
>>>>>>>> AIX.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> I created the following bug :
>>>>>>>>
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8193515
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> The compile error we get on AIX (using XLC 12.1) is :
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> === Output from failing command(s) repeated here ===
>>>>>>>>
>>>>>>>> * For target
>>>>>>>> support_native_java.desktop_libfontmanager_hb-ot-shape-complex-
>>>>>>
>>>>>> arabic.o:
>>>>>>>>
>>>>>>>> "
>>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot-
>>>>>>
>>>>>> shape-complex-arabic-fallback.hh",
>>>>>>>>
>>>>>>>> line 80.3: 1540-0218 (S) The call does not match any parameter list
>>>>>>>> for
>>>>>>>> "hb_stable_sort".
>>>>>>>>
>>>>>>>> "
>>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-
>>>>>>
>>>>>> private.hh",
>>>>>>>>
>>>>>>>> line 723.1: 1540-1283 (I) "template<class T, class T2>
>>>>>>>> hb_stable_sort(T *,
>>>>>>>> unsigned int, int (*)(const T *, const T *), T2 *)" is not a viable
>>>>>>>> candidate.
>>>>>>>>
>>>>>>>> "
>>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot-
>>>>>>
>>>>>> shape-complex-arabic-fallback.hh",
>>>>>>>>
>>>>>>>> line 80.43: 1540-0298 (I) Template argument deduction cannot be
>>>>>>
>>>>>> performed
>>>>>>>>
>>>>>>>> using the function "template int cmp(Type2) const".
>>>>>>>>
>>>>>>>> "
>>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-
>>>>>>
>>>>>> private.hh",
>>>>>>>>
>>>>>>>> line 748.1: 1540-1283 (I) "template<class T> hb_stable_sort(T *,
>>>>>
>>>>> unsigned
>>>>>>>>
>>>>>>>> int, int (*)(const T *, const T *))" is not a viable candidate.
>>>>>>>>
>>>>>>>> "
>>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot-
>>>>>>
>>>>>> shape-complex-arabic-fallback.hh",
>>>>>>>>
>>>>>>>> line 80.3: 1540-0215 (I) The wrong number of arguments has been
>>>>>>
>>>>>> specified
>>>>>>>>
>>>>>>>> for "template<class T> hb_stable_sort(T *, unsigned int, int
>>>>>>>> (*)(const T
>>>>>
>>>>> *,
>>>>>>>>
>>>>>>>> const T *))".
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> "
>>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot-
>>>>>>
>>>>>> shape-complex-arabic-fallback.hh",
>>>>>>>>
>>>>>>>> line 129.3: 1540-0218 (S) The call does not match any parameter list
>>>>>>>> for
>>>>>>>> "hb_stable_sort".
>>>>>>>>
>>>>>>>> "
>>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-
>>>>>>
>>>>>> private.hh",
>>>>>>>>
>>>>>>>> line 723.1: 1540-1283 (I) "template<class T, class T2>
>>>>>>>> hb_stable_sort(T *,
>>>>>>>> unsigned int, int (*)(const T *, const T *), T2 *)" is not a viable
>>>>>>>> candidate.
>>>>>>>>
>>>>>>>> "
>>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot-
>>>>>>
>>>>>> shape-complex-arabic-fallback.hh",
>>>>>>>>
>>>>>>>> line 129.55: 1540-0298 (I) Template argument deduction cannot be
>>>>>>
>>>>>> performed
>>>>>>>>
>>>>>>>> using the function "template int cmp(Type2) const".
>>>>>>>>
>>>>>>>> "
>>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-
>>>>>>
>>>>>> private.hh",
>>>>>>>>
>>>>>>>> line 748.1: 1540-1283 (I) "template<class T> hb_stable_sort(T *,
>>>>>
>>>>> unsigned
>>>>>>>>
>>>>>>>> int, int (*)(const T *, const T *))" is not a viable candidate.
>>>>>>>>
>>>>>>>> "
>>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot-
>>>>>>
>>>>>> shape-complex-arabic-fallback.hh",
>>>>>>>>
>>>>>>>> line 129.3: 1540-0215 (I) The wrong number of arguments has been
>>>>>>
>>>>>> specified
>>>>>>>>
>>>>>>>> for "template<class T> hb_stable_sort(T *, unsigned int, int
>>>>>>>> (*)(const T
>>>>>
>>>>> *,
>>>>>>>>
>>>>>>>> const T *))".
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> The compilation "complains" about the hb_stable_sort template used
>>>>>
>>>>> in
>>>>>>>>
>>>>>>>> hb-ot-shape-complex-arabic-fallback.hh . After looking a bit into
>>>>>>>> this ,
>>>>>>>> the third parameter
>>>>>>>>
>>>>>>>> OT::GlyphID::cmp
>>>>>>>>
>>>>>>>> of
>>>>>>>>
>>>>>>>> hb_stable_sort (&glyphs[0], num_glyphs, OT::GlyphID::cmp,
>>>>>>>> &substitutes[0]);
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> seems to trigger this XLC 12 issue .
>>>>>>>>
>>>>>>>> XLC 12 does not like the fact that we have two cmp functions (one a
>>>>>>>> template) in
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> hb-open-type-private.hh :
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> 610 template<typename Type, unsigned int Size>
>>>>>>>>
>>>>>>>> 611 struct IntType
>>>>>>>>
>>>>>>>> 612 {
>>>>>>>>
>>>>>>>> ....
>>>>>>>>
>>>>>>>> 617 static inline int cmp (const IntType<Type,Size> *a, const
>>>>>>>> IntType<Type,Size> *b) { return b->cmp (*a); }
>>>>>>>>
>>>>>>>> 622
>>>>>>>>
>>>>>>>> 623 template<typename Type2>
>>>>>>>>
>>>>>>>> 624 inline int cmp (Type2 a) const
>>>>>>>>
>>>>>>>> 625 {
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ( GlyphID is an IntType )
>>>>>>>>
>>>>>>>> This looks like an XLC bug, however it is pretty easy to workaround
>>>>>>>> it by
>>>>>>>> using a helper compare-function with a unique name (issue with cmp
>>>>>>>> is
>>>>>>
>>>>>> that
>>>>>>>>
>>>>>>>> it is not unique, that confuses XLC ).
>>>>>>>>
>>>>>>>> See this webrev :
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8193515/
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Please review it.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks, Matthias
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic