RFR (M): 8024265: Enable new build on AIX (top level part)
volker.simonis at gmail.com
Mon Sep 9 07:32:09 PDT 2013
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'
> 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
> 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
> 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 *], ) 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.
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?
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the ppc-aix-port-dev