RE: [nvptx] Pass -m32/-m64 to host_compiler if it has multilib support

[Prathamesh Kulkarni <prathameshk@nvidia.com>]

[-- Attachment #1: Type: text/plain, Size: 7661 bytes --]

> -----Original Message-----
> From: Thomas Schwinge <tschwinge@baylibre.com>
> Sent: Monday, September 9, 2024 8:50 PM
> To: Prathamesh Kulkarni <prathameshk@nvidia.com>; Richard Biener
> <rguenther@suse.de>
> Cc: Andrew Pinski <pinskia@gmail.com>; gcc-patches@gcc.gnu.org; Jakub
> Jelinek <jakub@redhat.com>
> Subject: RE: [nvptx] Pass -m32/-m64 to host_compiler if it has
> multilib support
> 
> External email: Use caution opening links or attachments
> 
> 
> Hi Prathamesh!
Hi Thomas,
> 
> On 2024-09-09T06:31:18+0000, Prathamesh Kulkarni
> <prathameshk@nvidia.com> wrote:
> >> -----Original Message-----
> >> From: Thomas Schwinge <tschwinge@baylibre.com>
> >> Sent: Friday, September 6, 2024 2:31 PM On 2024-08-
> 16T15:36:29+0000,
> >> Prathamesh Kulkarni <prathameshk@nvidia.com> wrote:
> >> >> > Am 13.08.2024 um 17:48 schrieb Thomas Schwinge
> >> >> <tschwinge@baylibre.com>:
> >> >> > On 2024-08-12T07:50:07+0000, Prathamesh Kulkarni
> >> >> <prathameshk@nvidia.com> wrote:
> >> >> >> 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).
> 
> >> > --- 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");
> >> >  }
> >>
> >> As none of the current offload compilers is set up of ILP32, I
> >> suggest we continue to pass '-foffload-abi=ilp32' without
> >> '-foffload-abi-host- opts=[...]' -- the 'mkoffload's in that case
> >> should get to the point where the latter is used.
> 
> Oh...  I was wrong with the latter item: I failed to see that the
> 'mkoffload's still do 'compile_native' even if they don't create an
> actual offload image, sorry!
> 
> > Um, would that still possibly result in arch mismatch for host
> objects and xnvptx-none.o if we don't pass host ABI opts for ILP32 ?
> > For eg, if the host compiler defaults to 64-bit code-gen (and user
> > requests for 32-bit code gen on host), and we avoid passing host ABI
> opts for -foffload-abi=ilp32, it will generate 64-bit xnvptx-none.o
> (corresponding to empty ptx_cfile_name), while rest of the host
> objects will be 32-bit, or am I misunderstanding ?
> 
> You're quite right -- my fault.
> 
> > The attached patch avoids passing -foffload-abi-host-opts if -
> foffload-abi=ilp32.
> 
> So, sorry for the back and forth.  I think we now agree that we do
> need '-foffload-abi-host-opts=[...]' specified in call cases (as you
> originally had), and then again unconditionally use
> 'offload_abi_host_opts' in the 'mkoffload's' 'compile_native'
> functions.
Done in the attached patch, thanks.
> 
> > Could you please test the patch for gcn backend ?
> 
> I'll do that.
> 
> > [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
> 
> "its", by the way.  ;-)
Fixed 😊
> 
> > to host_compiler.
> 
> > --- a/gcc/common.opt
> > +++ b/gcc/common.opt
> 
> > +foffload-abi-host-opts=
> > +Common Driver Joined MissingArgError(option missing after %qs)
> > +-foffload-abi-host-opts=<options> Specify host ABI options.
> > +
> 
> Still need TAB between '-foffload-abi-host-opts=<options>' and its
> help text.
Done.
> 
> > --- a/gcc/config/gcn/mkoffload.cc
> > +++ b/gcc/config/gcn/mkoffload.cc
> 
> > @@ -998,6 +996,14 @@ main (int argc, char **argv)
> >                        "unrecognizable argument of option %<" STR
> "%>");
> >       }
> >  #undef STR
> > +      else if (startswith (argv[i], "-foffload-abi-host-opts="))
> > +     {
> > +       if (offload_abi_host_opts)
> > +         fatal_error (input_location,
> > +                      "-foffload-abi-host-opts specified multiple
> > + times");
> 
> ACK, but again '%<-foffload-abi-host-opts%>', please.  (May also use
> another '#define STR "[...]"' for the duplicated string, but I don't
> care.)
Sorry, missed this earlier, fixed.
> 
> > --- a/gcc/config/nvptx/mkoffload.cc
> > +++ b/gcc/config/nvptx/mkoffload.cc
> 
> > @@ -721,6 +718,14 @@ main (int argc, char **argv)
> >                        "unrecognizable argument of option " STR);
> >       }
> >  #undef STR
> > +      else if (startswith (argv[i], "-foffload-abi-host-opts="))
> > +     {
> > +       if (offload_abi_host_opts)
> > +         fatal_error (input_location,
> > +                      "-foffload-abi-host-opts specified multiple
> > + times");
> 
> Likewise.
> 
> > --- a/gcc/lto-wrapper.cc
> > +++ b/gcc/lto-wrapper.cc
> > @@ -484,6 +484,7 @@ merge_and_complain (vec<cl_decoded_option>
> > &decoded_options,
> >
> >
> >       case OPT_foffload_abi_:
> > +     case OPT_foffload_abi_host_opts_:
> >         if (existing_opt == -1)
> >           decoded_options.safe_push (*foption);
> >         else if (foption->value !=
> > decoded_options[existing_opt].value)
> > @@ -745,6 +746,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:
> 
> I'm not too familiar with this code, but that now looks right to me.
> 
> > --- a/gcc/opts.cc
> > +++ b/gcc/opts.cc
> > @@ -3070,11 +3070,12 @@ 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
> > -      error_at (loc, "%<-foffload-abi%> option can be specified
> only for "
> > -             "offload compiler");
> > +      error_at (loc, "%qs option can be specified only for "
> > +             "offload compiler", arg);
> >  #endif
> >        break;
> 
> With this, using '-foffload-abi=ilp32' with the host compiler results
> in:
> 
>     cc1: error: ‘ilp32’ option can be specified only for offload
> compiler
> 
> ..., or for '-foffload-abi-host-opts=-m64' in:
> 
>     xgcc: error: ‘-m64’ option can be specified only for offload
> compiler
> 
> ..., so 'arg' is only the option argument, not the whole string.
Ah, didn't realize that, sorry. Fixed.
> 
> And, incidentally, 'cc1' vs. 'xgcc' means without vs. with 'Driver'
> option property (re your 'gcc/common.opt' change).  Which should it
> be?
> '-foffload-abi=[...]' currently doesn't have 'Driver', so probably '-
> foffload-abi-host-opts=[...]' also shouldn't?
Indeed, removed Driver from -foffload-abi-host-opts, thanks.
> 
> With those small items addressed, the patch looks good to me, thanks!
> (..., and I'll still test GCN offloading.)
Thanks, I have tested libgomp for aarch64/nvptx offloading.
Is it OK to commit (if testing at your end also passes on gcn) ?

Signed-off-by: Prathamesh Kulkarni <prathameshk@nvidia.com>

Thanks,
Prathamesh
> 
> 
> Grüße
>  Thomas

[-- Attachment #2: p-165-7.txt --]
[-- Type: text/plain, Size: 8239 bytes --]

[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 its value
to host_compiler.

gcc/ChangeLog:
	* common.opt (foffload-abi-host-opts): New option.
	* config/aarch64/aarch64.cc (aarch64_offload_options): Pass
	-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 (offload_abi_host_opts): Define.
	(compile_native): Append offload_abi_host_opts to argv_obstack.
	(main): Handle option -foffload-abi-host-opts.
	* config/gcn/mkoffload.cc (offload_abi_host_opts): Define.
	(compile_native): Append offload_abi_host_opts to argv_obstack.
	(main): Handle option -foffload-abi-host-opts.
	* lto-wrapper.cc (merge_and_complain): Handle
	-foffload-abi-host-opts.
	(append_compiler_options): Likewise.
	* opts.cc (common_handle_option): Likewise.

Signed-off-by: Prathamesh Kulkarni <prathameshk@nvidia.com>

diff --git a/gcc/common.opt b/gcc/common.opt
index ea39f87ae71..d270e524ff4 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 Joined MissingArgError(option missing after %qs)
+-foffload-abi-host-opts=<options>	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 6a3f1a23a9f..6ccf08d1cc0 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -19000,9 +19000,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/gcn/mkoffload.cc b/gcc/config/gcn/mkoffload.cc
index b8d981878ed..345bbf7709c 100644
--- a/gcc/config/gcn/mkoffload.cc
+++ b/gcc/config/gcn/mkoffload.cc
@@ -133,6 +133,8 @@ static const char *gcn_dumpbase;
 static struct obstack files_to_cleanup;
 
 enum offload_abi offload_abi = OFFLOAD_ABI_UNSET;
+const char *offload_abi_host_opts = NULL;
+
 uint32_t elf_arch = EF_AMDGPU_MACH_AMDGCN_GFX900;  // Default GPU architecture.
 uint32_t elf_flags = EF_AMDGPU_FEATURE_SRAMECC_UNSUPPORTED_V4;
 
@@ -819,17 +821,10 @@ compile_native (const char *infile, const char *outfile, const char *compiler,
   obstack_ptr_grow (&argv_obstack, gcn_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 (!offload_abi_host_opts)
+    fatal_error (input_location,
+		 "%<-foffload-abi-host-opts%> not specified.");
+  obstack_ptr_grow (&argv_obstack, offload_abi_host_opts);
   obstack_ptr_grow (&argv_obstack, infile);
   obstack_ptr_grow (&argv_obstack, "-c");
   obstack_ptr_grow (&argv_obstack, "-o");
@@ -998,6 +993,15 @@ main (int argc, char **argv)
 			 "unrecognizable argument of option %<" STR "%>");
 	}
 #undef STR
+      else if (startswith (argv[i], "-foffload-abi-host-opts="))
+	{
+	  if (offload_abi_host_opts)
+	    fatal_error (input_location,
+			 "%<-foffload-abi-host-opts%> specified "
+			 "multiple times");
+	  offload_abi_host_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/i386/i386-options.cc b/gcc/config/i386/i386-options.cc
index f79257cc764..55e0210260f 100644
--- a/gcc/config/i386/i386-options.cc
+++ b/gcc/config/i386/i386-options.cc
@@ -3680,8 +3680,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..df16ee64736 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 *offload_abi_host_opts = NULL;
 
 /* Delete tempfiles.  */
 
@@ -607,17 +608,10 @@ 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 (!offload_abi_host_opts)
+    fatal_error (input_location,
+		 "%<-foffload-abi-host-opts%> not specified.");
+  obstack_ptr_grow (&argv_obstack, offload_abi_host_opts);
   obstack_ptr_grow (&argv_obstack, infile);
   obstack_ptr_grow (&argv_obstack, "-c");
   obstack_ptr_grow (&argv_obstack, "-o");
@@ -721,6 +715,15 @@ main (int argc, char **argv)
 			 "unrecognizable argument of option " STR);
 	}
 #undef STR
+      else if (startswith (argv[i], "-foffload-abi-host-opts="))
+	{
+	  if (offload_abi_host_opts)
+	    fatal_error (input_location,
+			 "%<-foffload-abi-host-opts%> specified "
+			 "multiple times");
+	  offload_abi_host_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 08579bc83e6..0bf8bae27f5 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -17330,9 +17330,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");
 }
 
 \f
diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc
index c07765b37a2..7de045da9b9 100644
--- a/gcc/lto-wrapper.cc
+++ b/gcc/lto-wrapper.cc
@@ -484,6 +484,7 @@ merge_and_complain (vec<cl_decoded_option> &decoded_options,
  
 
 	case OPT_foffload_abi_:
+	case OPT_foffload_abi_host_opts_:
 	  if (existing_opt == -1)
 	    decoded_options.safe_push (*foption);
 	  else if (foption->value != decoded_options[existing_opt].value)
@@ -745,6 +746,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 fc6abf6f582..a78f73e57e3 100644
--- a/gcc/opts.cc
+++ b/gcc/opts.cc
@@ -3070,11 +3070,14 @@ 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
-      error_at (loc, "%<-foffload-abi%> option can be specified only for "
-		"offload compiler");
+      error_at (loc,
+		"%qs option can be specified only for offload compiler",
+		(code == OPT_foffload_abi_) ? "-foffload-abi"
+					    : "-foffload-abi-host-opts");
 #endif
       break;