> -----Original Message-----
> From: Richard Biener <rguent...@suse.de>
> Sent: Tuesday, August 13, 2024 10:06 PM
> To: Thomas Schwinge <tschwi...@baylibre.com>
> Cc: Prathamesh Kulkarni <prathame...@nvidia.com>; Andrew Pinski
> <pins...@gmail.com>; gcc-patches@gcc.gnu.org; Jakub Jelinek
> <ja...@redhat.com>
> Subject: Re: [nvptx] Pass -m32/-m64 to host_compiler if it has
> multilib support
>
> External email: Use caution opening links or attachments
>
>
> > 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?
Well, my concern was if that'd change the behavior for TARGET_ILP32 ?
IIUC, currently for -foffload-abi=ilp32, mkoffload will create empty C file
for ptx_cfile_name (instead of munged ptx assembly since offloading will be
disabled),
and pass that to host compiler with -m32 option (in compile_native).
If we change -foffload-abi to specify ABI host opts, and pass
-foffload-abi=disable
for TARGET_ILP32 in TARGET_OFFLOAD_OPTIONS, mkoffload will no longer be able to
pass 32-bit ABI opts to host compiler, which may result in linker error (arch
mismatch?)
if the host object files are 32-bit ABI and xnvptx-none.o is 64-bit (assuming
the host
compiler is configured to generate 64-bit code-gen by default) ?
So, I thought to add another option -foffload-abi-host-opts to pass
host-specific ABI opts,
and keep -foffload-abi as-is to infer ABI type for enabling/disabling
offloading.
> >
> > 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)?
I think nvptx already supports m64 and ignores it.
From nvptx.opt:
m64
Target RejectNegative Mask(ABI64)
Ignored, but preserved for backward compatibility. Only 64-bit ABI is
supported.
Thanks,
Prathamesh
>
> 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
