RFR (M): 8024265: Enable new build on AIX (top level part)

Tue Sep 10 13:22:32 UTC 2013

On Tue, Sep 10, 2013 at 3:05 PM, Magnus Ihse Bursie <
magnus.ihse.bursie at oracle.com> wrote:

> Sorry, I missed your question. I can't speak for the aix-port repo, but
> I'm satisfied from the build point of view.
>
>
Thanks. I've added you as reviewer.

> /Magnus
>
> 10 sep 2013 kl. 14:22 skrev Volker Simonis <volker.simonis at gmail.com>:
>
>
>
>
> On Mon, Sep 9, 2013 at 4:32 PM, Volker Simonis <volker.simonis at gmail.com>wrote:
>
>> Hi Magnus,
>>
>> thanks again for the review. Please see my comments inline:
>>
>>
>> On Mon, Sep 9, 2013 at 12:23 PM, Magnus Ihse Bursie <
>> magnus.ihse.bursie at oracle.com> wrote:
>>
>>>  Hi Volker,
>>>
>>> Some more comments inlined.
>>>
>>>  OK, common/autoconf/build-aux/autoconf-config.guess was too old and
>>> didn't knew about AIX 7 so it returned the default AIX fallback which is
>>> 'rs6000-ibm-aix'. I have now fixed 'autoconf-config.guess' to know about
>>> AIX 7 (a one character change which is already in autoconf-2.69 so we won't
>>> have problems if you should ever update autoconf-config.guess).
>>>
>>>
>>> Hmm... While more elegant, the idea was that autoconf-config.guess
>>> should be a strict copy of the config.guess file from the autoconf package.
>>> The whole idea of wrapping it in a custom config.guess was to avoid
>>> "forking" the autoconf config.guess, with small changes (like this) that
>>> would be hard to track and might get lost if we update to a newer version
>>> from the upstream autoconf file.
>>>
>>> That being said, since we settled on autoconf 2.69, we really should
>>> update the file to config.guess from autoconf-2.69. I've started a test run
>>> on our internal test system with the config.guess from 2.69 to see if it
>>> breaks any existing platforms. If not, I suggest we do a fast integration
>>> of the new config.guess.
>>>
>>> Can you hold on with your changes until a new config.guess comes in?
>>> Even if you're just making a small change that will be reverted soon after,
>>> I think it would set a unfortunate precedent.
>>>
>>
>> That's OK for me. Actually my change still works without the changes in
>> "config.guess" on AIX 5.3. And it will automatically start working once we
>> get the new "config.guess".
>>
>>
>>>  Ahrgh, all these proud compilers with their own ways of expressing the
>>>> same functionality. :( I assume that you are using the COMP_MODE_OPTION in
>>>> the jdk projct? I couldn't find any references to it in the Hotspot build
>>>> changes, and otherwise there seems to be no reason to export it in the
>>>> spec.gmk file.
>>>>
>>>>
>>>  Yes, exactly. It is used in 'jdk/makefiles/GensrcX11Wrappers.gmk'
>>> (MEMORY_MODEL_FLAG="$(COMP_MODE_OPTION)$*").
>>>
>>> Once again the X11 wrappers. We should really make an effort and get rid
>>> of them. :-/
>>>
>>>
>>> I don't mind:)
>>
>>>    I fully agree with the criticism on the name:) After we already have
>>> 'COMPILER_SUPPORTS_TARGET_BITS_FLAG' I've simply renamed it to
>>> 'COMPILER_TARGET_BITS_FLAG'. I think that's much more appropriate and if
>>> you don't like it we should ask the one who invented
>>> 'COMPILER_SUPPORTS_TARGET_BITS_FLAG' :) And I had to set
>>> 'COMPILER_TARGET_BITS_FLAG' a little earlier  such that is availabel in
>>> PLATFORM_SET_COMPILER_TARGET_BITS_FLAGS.
>>>
>>> Sounds good! Nice aligning with the existing macro.
>>>
>>> However, you forgot to change the name in spec.gmk.in. (At least in the
>>> webrev you published.)
>>>
>>
>> Good catch! I didn't realize it because the build in the stage
>> repositories currently doesn’t reach the X11 wrappers.
>> I've changed it and moved it near the definition of
>> 'COMPILER_SUPPORTS_TARGET_BITS_FLAG'
>>
>>
>>>   2. After you pointed out that setting '-q64' as extra flags on the
>>> configure command line is not the way it is supposed to work I recalled
>>> that we also have this problem on older Linux/PPC64 boxes (e.g. SLES 10)
>>> where the default compiler produces 32-bit objects by default. To fix this
>>> problem as well, I've inserted a call to
>>> PLATFORM_SET_COMPILER_TARGET_BITS_FLAGS followed by a second call to
>>> AC_CHECK_SIZEOF([int *], [1111]) in the case where we would otherwise have
>>> bailed out because the "TESTED_TARGET_CPU_BITS" differs from the actual
>>> "OPENJDK_TARGET_CPU_BITS". I think this change should not affect any
>>> existing platforms because it will only be triggered where we woould have
>>> bailed out with an error anyway.
>>>
>>>  Also, the workaround for autoconf bug
>>> http://lists.gnu.org/archive/html/autoconf/2010-07/msg00004.html in
>>> AC_CHECK_SIZEOF isn't needed any more now that we require at least
>>> autoconf-2.69 because the problem was fixed in 2.67. And if I looked at it
>>> more carefully I must say that I don't understand the workaround at all. In
>>> my opinion, the test "x$SIZEOF_INT_P" != "x$ac_cv_sizeof_int_p" will always
>>> fail, because the AC_CHECK_SIZEOF macro only writes a define for
>>> SIZEOF_INT_P into "confdefs.h" (as can be seen in generated-configure.h)
>>> but it never defines it in the shell. And defining SIZEOF_INT_P in the
>>> configure shell script wouldn't help if the define written by the
>>> AC_CHECK_SIZEOF macro was wrong (as described in the bug). So Ithink the
>>> best is to remove the workaround and use "ac_cv_sizeof_int_p" in the places
>>> where we used AC_CHECK_SIZEOF before.
>>>
>>>
>>> Good to get rid of the old workaround. I agree, it looks kind of weird.
>>> I think I might have been behind some of the weirdness; I think I
>>> interpreted the autoconf documentation as if it should assign a variable
>>> SIZEOF_INT_P in the configure script, and that the $ac_* variables were
>>> internal variables that should not be directly accessed. In the current
>>> documentation, at least, the $ac_cv_sizeof* macro is officially mentioned
>>> so it should be safe to use.
>>>
>>> However, your second relies on some internal autoconf magic, by
>>> unsetting variables and defines. We've tried to avoid that, but at times
>>> there were no choice. Since we're about to fail anyway, and the code is
>>> more in place for future, strange platforms, it's probably no harm.
>>>
>>>
>> Exactly.
>>
>> Is it OK if I push it now to
>> http://hg.openjdk.java.net/ppc-aix-port/ppc-aix-port/stage or is there
>> anything you want to test first?
>>
>
> Vladimir will push the changes because he also needs to re-generate the
> closed config files (thanks Vladimir).
>
> Here's the final webrev:
>
> http://cr.openjdk.java.net/~simonis/webrevs/8024265.v3
>
> @Vladimir: I used autoconf 2.69 to create the webrev, so if you recreate
> the config files the diff to 'common/autoconf/generated-configure.sh'
> should be only the time-stamp field.
>
> Regards,
> Volker
>
>
>>
>>> /Magnus
>>>
>>
>>
>