Looks good to me. /Magnus
> 18 feb. 2019 kl. 15:37 skrev Doerr, Martin <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>; > 'build-dev@openjdk.java.net' <build-dev@openjdk.java.net> > Cc: Doerr, Martin <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/ > > > Best Regards, Matthias > > > > From: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> > Sent: Montag, 18. Februar 2019 11:18 > To: Baesken, Matthias <matthias.baes...@sap.com>; > 'build-dev@openjdk.java.net' <build-dev@openjdk.java.net> > Cc: Doerr, Martin <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 > > > >