RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]

Magnus Ihse Bursie ihse at openjdk.org
Tue Apr 16 08:37:02 UTC 2024


On Mon, 15 Apr 2024 13:10:22 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

>> It was too bad that I did not see and review this change in the makefiles. :-(
>> 
>> While you guys could have gone either way, I strongly dislike the choice to include a redefinition in the makefiles. If this really should be done, we should introduced a new variable to carry such changes, instead of piggybacking it with the OS defines. :-( But, I don't think it should be done at all.
>> 
>> There are several reasons why this is a inferior solution:
>> 
>> 1) It does not follow prior examples. We have tried hard before not do things like this, but rather pass flags as defines (e.g. `-DREDEFINE_ALLOCA` had been better)
>> 2) It does not scale. If we start in effect allowing code in the command line, there is no clear limit anymore what should be placed in the source code files and what should be placed on the command line.
>> 3) It messes up command lines. Keeping command lines as short as reasonable possible is a goal we try to strive for. In this case, there is also the `'` inside them (which I don't understand why), which is just begging for quoting/escaping problems, making command lines hard to copy/paste, send to different systems (like logging) etc.
>> 
>> I'd really like to see a follow-up PR that moves this away from the command line define and into a source code file instead.
>
> Can we unconditionally `#include <alloca.h>` in all files which use `alloca`? Or does that disturb any platform?

I don't think it does. From the documentation I've checked, `#include <alloca.h>` is valid everywhere. I'm not sure why the fact that it is a compiler-builtin is relevant here. The man page for alloca on Linux says:


SYNOPSIS
       #include <alloca.h>

       void *alloca(size_t size);


which I take it as this is the formally correct way to use alloca. If it happens to work without the include file, that's just coincidence, and not a sign that we should remove `#include <alloca.h>` everywhere. @kimbarrett What do you say?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1566964500


More information about the build-dev mailing list