[OpenJDK 2D-Dev] RFR : 8193515 : AIX : new Harfbuzz 1.7.1 version fails to compile

Volker Simonis volker.simonis at gmail.com
Mon Dec 18 09:51:01 UTC 2017 

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 at 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 at oracle.com>; Volker Simonis
>> <volker.simonis at gmail.com>
>> Cc: Simonis, Volker <volker.simonis at sap.com>; 2d-dev at 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 at oracle.com]
>> > Sent: Donnerstag, 14. Dezember 2017 17:53
>> > To: Volker Simonis <volker.simonis at gmail.com>; Baesken, Matthias
>> > <matthias.baesken at sap.com>
>> > Cc: Simonis, Volker <volker.simonis at sap.com>; 2d-dev at 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 at 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
>

More information about the 2d-dev mailing list