> Am 13.08.2024 um 17:48 schrieb Thomas Schwinge <tschwi...@baylibre.com>:
>
> Hi Prathamesh!
>
> On 2024-08-12T07:50:07+0000, Prathamesh Kulkarni <prathame...@nvidia.com>
> wrote:
>>> From: Thomas Schwinge <tschwi...@baylibre.com>
>>> Sent: Friday, August 9, 2024 12:55 AM
>
>>> On 2024-08-08T06:46:25-0700, Andrew Pinski <pins...@gmail.com> wrote:
>>>> On Thu, Aug 8, 2024 at 6:11 AM Prathamesh Kulkarni
>>>> <prathame...@nvidia.com> wrote:
>>>>> After differing NUM_POLY_INT_COEFFS fix for AArch64/nvptx
>>> offloading, the following minimal test:
>>>
>>> First, thanks for your work on enabling this! I will say that I had
>>> the plan to re-engage with Nvidia to hire us (as initial implementors
>>> of GCC/nvptx offloading) to make AArch64/nvptx offloading work, but
>>> now that Nvidia has its own GCC team, that's great that you're able to
>>> work on this yourself! :-)
>>>
>>> Please CC me for GCC/nvptx issues for (at least potentially...) faster
>>> response times.
>> Thanks, will do 😊
>
> Heh, so much for "potentially": I'm not able to spend a lot of time on
> this right now, as I shall soon be out of office. Quickly:
>
>>>>> compiled with -fopenmp -foffload=nvptx-none now fails with:
>>>>> gcc: error: unrecognized command-line option '-m64'
>>>>> nvptx mkoffload: fatal error: ../install/bin/gcc returned 1 exit
>>> status compilation terminated.
>>>
>>> Heh. Yeah...
>>>
>>>>> As mentioned in RFC email, this happens because
>>>>> nvptx/mkoffload.cc:compile_native passes -m64/-m32 to host compiler
>>> depending on whether offload_abi is OFFLOAD_ABI_LP64 or
>>> OFFLOAD_ABI_ILP32, and aarch64 backend doesn't recognize these
>>> options.
>
>>> So, my idea is: instead of the current strategy that the host
>>> 'TARGET_OFFLOAD_OPTIONS' synthesizes '-foffload-abi=lp64' etc., which
>>> the 'mkoffload's then interpret and re-synthesize '-m64' etc. -- how
>>> about we instead directly tell the 'mkoffload's the relevant ABI
>>> options? That is, 'TARGET_OFFLOAD_OPTIONS' instead synthesizes '-
>>> foffload-abi=-m64'
>>> etc., which the 'mkoffload's can then readily use. Could you please
>>> give that a try, and/or does anyone see any issues with that approach?
>>>
>>> And use something like '-foffload-abi=disable' to replace the current:
>>>
>>> /* PR libgomp/65099: Currently, we only support offloading in 64-
>>> bit
>>> configurations. */
>>> if (offload_abi == OFFLOAD_ABI_LP64)
>>> {
>>>
>>> (As discussed before, this should be done differently altogether, but
>>> that's for another day.)
>> Sorry, I don't quite follow. Currently we enable offloading if offload_abi
>> == OFFLOAD_ABI_LP64,
>> which is synthesized from -foffload-abi=lp64. If we change -foffload-abi to
>> instead specify
>> host-specific ABI opts, I guess mkoffload will still need to somehow figure
>> out which ABI is used,
>> so it can disable offloading for 32-bit ? I suppose we could adjust
>> TARGET_OFFLOAD_OPTIONS for each
>> host to pass -foffload-abi=disable if TARGET_ILP32 is set and offload target
>> is nvptx, but not sure
>> if that'd be correct ?
>
> Basically, yes. My idea was that all 'TARGET_OFFLOAD_OPTIONS'
> implementations return either the correct host flags to be used by the
> 'mkoffload's (the case that offloading is supported for the current host
> flags/ABI configuration), or otherwise return '-foffload-abi=disable'.
> For example (untested):
>
>> char *
>> ix86_offload_options (void)
>> {
>> if (TARGET_LP64)
>> - return xstrdup ("-foffload-abi=lp64");
>> + return xstrdup ("-foffload-abi=-m64");
>> - return xstrdup ("-foffload-abi=ilp32");
>> + return xstrdup ("-foffload-abi=disable");
>> }
>
> That is, only for 'TARGET_LP64' offloading is supported, and via
> '-foffload-abi=-m64' the 'mkoffload's know that they need to specify
> '-m64'. For other host flags/ABI configuration, the 'mkoffload's see
> '-foffload-abi=disable' and thus disable offload code generation
> (replacing the current 'if (offload_abi == OFFLOAD_ABI_LP64)' in
> 'mkoffload').
>
>> In the attached patch
>
> Yes, that's going in the right direction, thanks!
>
>> I added another option -foffload-abi-host-opts to specify host abi
>> opts, and leave -foffload-abi to specify if ABI is 32/64 bit which mkoffload
>> can use to
>> enable/disable offloading (as before).
>
> I'm not sure however, if this additional option is really necessary?
>
> In case we're not happy to re-purpose the flag name
> '-foffload-abi=[...]', we could also rename that one to
> '-foffload-abi-host-opts=[...]'; the former is not user-exposed, so we
> may change it as necessary. (Or, in other words, go with your proposed
> '-foffload-abi-host-opts=[...]', but also remove '-foffload-abi=[...]' at
> the same time.)
>
>
> I'll be able to spend more time on this in two weeks.
Since we do not support 32 -> 64 bit offload wouldn’t the most pragmatic fix be
to recognize -m64 in the nvptx backend (and ignore it)?
Richard
>
> Grüße
> Thomas
>
>
>> [nvptx] Pass host specific ABI opts from mkoffload.
>>
>> The patch adds an option -foffload-abi-host-opts, which
>> is set by host in TARGET_OFFLOAD_OPTIONS, and mkoffload then passes it's
>> value
>> to host_compiler.
>>
>> gcc/ChangeLog:
>> * common.opt (foffload-abi-host-opts): New option.
>> * config/aarch64/aarch64.cc (aarch64_offload_options): Set
>> -foffload-abi-host-opts.
>> * config/i386/i386-opts.cc (ix86_offload_options): Likewise.
>> * config/rs6000/rs6000.cc (rs6000_offload_options): Likewise.
>> * config/nvptx/mkoffload.cc (host_abi_opts): Define.
>> (compile_native): Append host_abi_opts to argv_obstack.
>> (main): Handle option -foffload-abi-host-opts.
>> * lto-wrapper.cc (append_compiler_options): Handle
>> -foffload-abi-host-opts.
>> * opts.cc (common_handle_option): Likewise.
>>
>> Signed-off-by: Prathamesh Kulkarni <prathame...@nvidia.com>
>>
>> diff --git a/gcc/common.opt b/gcc/common.opt
>> index ea39f87ae71..d1a9efb9513 100644
>> --- a/gcc/common.opt
>> +++ b/gcc/common.opt
>> @@ -2361,6 +2361,10 @@ Enum(offload_abi) String(ilp32)
>> Value(OFFLOAD_ABI_ILP32)
>> EnumValue
>> Enum(offload_abi) String(lp64) Value(OFFLOAD_ABI_LP64)
>>
>> +foffload-abi-host-opts=
>> +Common Driver Joined MissingArgError(option or option=abi missing after %qs)
>> +-foffload-abi-host-opts=<options>=<abi> Specify host abi options.
>> +
>> fomit-frame-pointer
>> Common Var(flag_omit_frame_pointer) Optimization
>> When possible do not generate stack frames.
>> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
>> index 2ac5a22c848..7418cb1fb69 100644
>> --- a/gcc/config/aarch64/aarch64.cc
>> +++ b/gcc/config/aarch64/aarch64.cc
>> @@ -18999,9 +18999,9 @@ static char *
>> aarch64_offload_options (void)
>> {
>> if (TARGET_ILP32)
>> - return xstrdup ("-foffload-abi=ilp32");
>> + return xstrdup ("-foffload-abi=ilp32
>> -foffload-abi-host-opts=-mabi=ilp32");
>> else
>> - return xstrdup ("-foffload-abi=lp64");
>> + return xstrdup ("-foffload-abi=lp64
>> -foffload-abi-host-opts=-mabi=lp64");
>> }
>>
>> static struct machine_function *
>> diff --git a/gcc/config/i386/i386-options.cc
>> b/gcc/config/i386/i386-options.cc
>> index 1c8f7835af2..bd960674e5d 100644
>> --- a/gcc/config/i386/i386-options.cc
>> +++ b/gcc/config/i386/i386-options.cc
>> @@ -3669,8 +3669,8 @@ char *
>> ix86_offload_options (void)
>> {
>> if (TARGET_LP64)
>> - return xstrdup ("-foffload-abi=lp64");
>> - return xstrdup ("-foffload-abi=ilp32");
>> + return xstrdup ("-foffload-abi=lp64 -foffload-abi-host-opts=-m64");
>> + return xstrdup ("-foffload-abi=ilp32 -foffload-abi-host-opts=-m32");
>> }
>>
>> /* Handle "cdecl", "stdcall", "fastcall", "regparm", "thiscall",
>> diff --git a/gcc/config/nvptx/mkoffload.cc b/gcc/config/nvptx/mkoffload.cc
>> index 503b1abcefd..d5ca2386641 100644
>> --- a/gcc/config/nvptx/mkoffload.cc
>> +++ b/gcc/config/nvptx/mkoffload.cc
>> @@ -61,6 +61,7 @@ static const char *omp_requires_file;
>> static const char *ptx_dumpbase;
>>
>> enum offload_abi offload_abi = OFFLOAD_ABI_UNSET;
>> +const char *host_abi_opts = NULL;
>>
>> /* Delete tempfiles. */
>>
>> @@ -607,17 +608,9 @@ compile_native (const char *infile, const char
>> *outfile, const char *compiler,
>> obstack_ptr_grow (&argv_obstack, ptx_dumpbase);
>> obstack_ptr_grow (&argv_obstack, "-dumpbase-ext");
>> obstack_ptr_grow (&argv_obstack, ".c");
>> - switch (offload_abi)
>> - {
>> - case OFFLOAD_ABI_LP64:
>> - obstack_ptr_grow (&argv_obstack, "-m64");
>> - break;
>> - case OFFLOAD_ABI_ILP32:
>> - obstack_ptr_grow (&argv_obstack, "-m32");
>> - break;
>> - default:
>> - gcc_unreachable ();
>> - }
>> + if (!host_abi_opts)
>> + fatal_error (input_location, "-foffload-abi-host-opts not specified.");
>> + obstack_ptr_grow (&argv_obstack, host_abi_opts);
>> obstack_ptr_grow (&argv_obstack, infile);
>> obstack_ptr_grow (&argv_obstack, "-c");
>> obstack_ptr_grow (&argv_obstack, "-o");
>> @@ -721,6 +714,8 @@ main (int argc, char **argv)
>> "unrecognizable argument of option " STR);
>> }
>> #undef STR
>> + else if (startswith (argv[i], "-foffload-abi-host-opts="))
>> + host_abi_opts = argv[i] + strlen ("-foffload-abi-host-opts=");
>> else if (strcmp (argv[i], "-fopenmp") == 0)
>> fopenmp = true;
>> else if (strcmp (argv[i], "-fopenacc") == 0)
>> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
>> index 0bcc6a2d0ab..decdf49a1f5 100644
>> --- a/gcc/config/rs6000/rs6000.cc
>> +++ b/gcc/config/rs6000/rs6000.cc
>> @@ -17333,9 +17333,9 @@ static char *
>> rs6000_offload_options (void)
>> {
>> if (TARGET_64BIT)
>> - return xstrdup ("-foffload-abi=lp64");
>> + return xstrdup ("-foffload-abi=lp64 -foffload-abi-host-opts=-m64");
>> else
>> - return xstrdup ("-foffload-abi=ilp32");
>> + return xstrdup ("-foffload-abi=ilp32 -foffload-abi-host-opts=-m32");
>> }
>>
>>
>> diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc
>> index 6bfc96590a5..1ecc4997e5a 100644
>> --- a/gcc/lto-wrapper.cc
>> +++ b/gcc/lto-wrapper.cc
>> @@ -745,6 +745,7 @@ append_compiler_options (obstack *argv_obstack,
>> vec<cl_decoded_option> opts)
>> case OPT_fopenacc:
>> case OPT_fopenacc_dim_:
>> case OPT_foffload_abi_:
>> + case OPT_foffload_abi_host_opts_:
>> case OPT_fcf_protection_:
>> case OPT_fasynchronous_unwind_tables:
>> case OPT_funwind_tables:
>> diff --git a/gcc/opts.cc b/gcc/opts.cc
>> index 0b7b137c376..79118237ce4 100644
>> --- a/gcc/opts.cc
>> +++ b/gcc/opts.cc
>> @@ -3069,6 +3069,7 @@ common_handle_option (struct gcc_options *opts,
>> break;
>>
>> case OPT_foffload_abi_:
>> + case OPT_foffload_abi_host_opts_:
>> #ifdef ACCEL_COMPILER
>> /* Handled in the 'mkoffload's. */
>> #else
