[v3] LoongArch: add addr_global attribute

Message ID 9b6b0e68cfb7e87ae961ef8a7bb7987f534da19c.camel@xry111.site
State New
Headers
Series [v3] LoongArch: add addr_global attribute |

Commit Message

Xi Ruoyao July 29, 2022, 5:17 p.m. UTC
  Change v2 to v3:
- Disable section anchor for addr_global symbols.
- Use -O2 in test to make sure section anchor is disabled.

--

Background:
https://lore.kernel.org/loongarch/d7670b60-2782-4642-995b-7baa01779a66@loongson.cn/T/#e1d47e2fe185f2e2be8fdc0784f0db2f644119379

Question:  Do you have a better name than "addr_global"?

Alternatives:

1. Just use "-mno-explicit-relocs -mla-local-with-abs" for kernel
modules.  It's stupid IMO.
2. Implement a "-maddress-local-with-got" option for GCC and use it for
kernel modules.  It seems too overkill: we might create many unnecessary
GOT entries.
3. For all variables with a section attribute, consider it global.  It
may make sense, but I just checked x86_64 and riscv and they don't do
this.
4. Implement -mcmodel=extreme and use it for kernel modules.  To me
"extreme" seems really too extreme.
5. More hacks in kernel. (Convert relocations against .data..percpu with
objtool?  But objtool is not even implemented for LoongArch yet.)

Note: I'll be mostly AFK in the following week.  My attempt to finish
the kernel support for new relocs before going AFK now failed miserably
:(.

-- >8 --

A linker script and/or a section attribute may locate a local object in
some way unexpected by the code model, leading to a link failure.  This
happens when the Linux kernel loads a module with "local" per-CPU
variables.

Add an attribute to explicitly mark an variable with the address
unlimited by the code model so we would be able to work around such
problems.

gcc/ChangeLog:

	* config/loongarch/loongarch.cc (loongarch_attribute_table):
	New attribute table.
	(TARGET_ATTRIBUTE_TABLE): Define the target hook.
	(loongarch_handle_addr_global_attribute): New static function.
	(loongarch_classify_symbol): Return SYMBOL_GOT_DISP for
	SYMBOL_REF_DECL with addr_global attribute.
	(loongarch_use_anchors_for_symbol_p): New static function.
	(TARGET_USE_ANCHORS_FOR_SYMBOL_P): Define the target hook.
	* doc/extend.texi (Variable Attributes): Document new
	LoongArch specific attribute.

gcc/testsuite/ChangeLog:

	* gcc.target/loongarch/addr-global.c: New test.
---
 gcc/config/loongarch/loongarch.cc             | 61 +++++++++++++++++++
 gcc/doc/extend.texi                           | 17 ++++++
 .../gcc.target/loongarch/addr-global.c        | 28 +++++++++
 3 files changed, 106 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/loongarch/addr-global.c
  

Comments

Lulu Cheng July 30, 2022, 3:13 a.m. UTC | #1
在 2022/7/30 上午1:17, Xi Ruoyao 写道:
> Change v2 to v3:
> - Disable section anchor for addr_global symbols.
> - Use -O2 in test to make sure section anchor is disabled.
>
> --
>
> Background:
> https://lore.kernel.org/loongarch/d7670b60-2782-4642-995b-7baa01779a66@loongson.cn/T/#e1d47e2fe185f2e2be8fdc0784f0db2f644119379
>
> Question:  Do you have a better name than "addr_global"?

I think the name can be changed to "get_through_got".What do you think of it?

>
> Alternatives:
>
> 1. Just use "-mno-explicit-relocs -mla-local-with-abs" for kernel
> modules.  It's stupid IMO.
> 2. Implement a "-maddress-local-with-got" option for GCC and use it for
> kernel modules.  It seems too overkill: we might create many unnecessary
> GOT entries.
> 3. For all variables with a section attribute, consider it global.  It
> may make sense, but I just checked x86_64 and riscv and they don't do
> this.
> 4. Implement -mcmodel=extreme and use it for kernel modules.  To me
> "extreme" seems really too extreme.
> 5. More hacks in kernel. (Convert relocations against .data..percpu with
> objtool?  But objtool is not even implemented for LoongArch yet.)
>
> Note: I'll be mostly AFK in the following week.  My attempt to finish
> the kernel support for new relocs before going AFK now failed miserably
> :(.
>
> -- >8 --
>
> A linker script and/or a section attribute may locate a local object in
> some way unexpected by the code model, leading to a link failure.  This
> happens when the Linux kernel loads a module with "local" per-CPU
> variables.
>
> Add an attribute to explicitly mark an variable with the address
> unlimited by the code model so we would be able to work around such
> problems.
>

Others I think are ok.
  
Huacai Chen July 30, 2022, 6:03 a.m. UTC | #2
Hi, Lulu,

On Sat, Jul 30, 2022 at 11:13 AM Lulu Cheng <chenglulu@loongson.cn> wrote:
>
>
> 在 2022/7/30 上午1:17, Xi Ruoyao 写道:
>
> Change v2 to v3:
> - Disable section anchor for addr_global symbols.
> - Use -O2 in test to make sure section anchor is disabled.
>
> --
>
> Background:
> https://lore.kernel.org/loongarch/d7670b60-2782-4642-995b-7baa01779a66@loongson.cn/T/#e1d47e2fe185f2e2be8fdc0784f0db2f644119379
>
> Question:  Do you have a better name than "addr_global"?
>
> I think the name can be changed to "get_through_got". What do you think of it?
Is this the same thing as "movable" once we used internally?

Huacai
>
> Alternatives:
>
> 1. Just use "-mno-explicit-relocs -mla-local-with-abs" for kernel
> modules.  It's stupid IMO.
> 2. Implement a "-maddress-local-with-got" option for GCC and use it for
> kernel modules.  It seems too overkill: we might create many unnecessary
> GOT entries.
> 3. For all variables with a section attribute, consider it global.  It
> may make sense, but I just checked x86_64 and riscv and they don't do
> this.
> 4. Implement -mcmodel=extreme and use it for kernel modules.  To me
> "extreme" seems really too extreme.
> 5. More hacks in kernel. (Convert relocations against .data..percpu with
> objtool?  But objtool is not even implemented for LoongArch yet.)
>
> Note: I'll be mostly AFK in the following week.  My attempt to finish
> the kernel support for new relocs before going AFK now failed miserably
> :(.
>
> -- >8 --
>
> A linker script and/or a section attribute may locate a local object in
> some way unexpected by the code model, leading to a link failure.  This
> happens when the Linux kernel loads a module with "local" per-CPU
> variables.
>
> Add an attribute to explicitly mark an variable with the address
> unlimited by the code model so we would be able to work around such
> problems.
>
>
> Others I think are ok.
  
Chenghua Xu July 31, 2022, 9:05 a.m. UTC | #3
在 2022/7/30 上午1:17, Xi Ruoyao via Gcc-patches 写道:
> Change v2 to v3:
> - Disable section anchor for addr_global symbols.
> - Use -O2 in test to make sure section anchor is disabled.
>
> --
>
> Background:
> https://lore.kernel.org/loongarch/d7670b60-2782-4642-995b-7baa01779a66@loongson.cn/T/#e1d47e2fe185f2e2be8fdc0784f0db2f644119379
>
> Question:  Do you have a better name than "addr_global"?
>
> Alternatives:
>
> 1. Just use "-mno-explicit-relocs -mla-local-with-abs" for kernel
> modules.  It's stupid IMO.
> 2. Implement a "-maddress-local-with-got" option for GCC and use it for
> kernel modules.  It seems too overkill: we might create many unnecessary
> GOT entries.
> 3. For all variables with a section attribute, consider it global.  It
> may make sense, but I just checked x86_64 and riscv and they don't do
> this.
> 4. Implement -mcmodel=extreme and use it for kernel modules.  To me
> "extreme" seems really too extreme.
> 5. More hacks in kernel. (Convert relocations against .data..percpu with
> objtool?  But objtool is not even implemented for LoongArch yet.)
>
> Note: I'll be mostly AFK in the following week.  My attempt to finish
> the kernel support for new relocs before going AFK now failed miserably
> :(.
>
> -- >8 --
>
> A linker script and/or a section attribute may locate a local object in
> some way unexpected by the code model, leading to a link failure.  This
> happens when the Linux kernel loads a module with "local" per-CPU
> variables.
>
> Add an attribute to explicitly mark an variable with the address
> unlimited by the code model so we would be able to work around such
> problems.
>
> gcc/ChangeLog:
>
> 	* config/loongarch/loongarch.cc (loongarch_attribute_table):
> 	New attribute table.
> 	(TARGET_ATTRIBUTE_TABLE): Define the target hook.
> 	(loongarch_handle_addr_global_attribute): New static function.
> 	(loongarch_classify_symbol): Return SYMBOL_GOT_DISP for
> 	SYMBOL_REF_DECL with addr_global attribute.
> 	(loongarch_use_anchors_for_symbol_p): New static function.
> 	(TARGET_USE_ANCHORS_FOR_SYMBOL_P): Define the target hook.
> 	* doc/extend.texi (Variable Attributes): Document new
> 	LoongArch specific attribute.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.target/loongarch/addr-global.c: New test.
> ---
>   gcc/config/loongarch/loongarch.cc             | 61 +++++++++++++++++++
>   gcc/doc/extend.texi                           | 17 ++++++
>   .../gcc.target/loongarch/addr-global.c        | 28 +++++++++
>   3 files changed, 106 insertions(+)
>   create mode 100644 gcc/testsuite/gcc.target/loongarch/addr-global.c
>
> diff --git a/gcc/config/loongarch/loongarch.cc b/gcc/config/loongarch/loongarch.cc
> index 79687340dfd..db6f84d4e66 100644
> --- a/gcc/config/loongarch/loongarch.cc
> +++ b/gcc/config/loongarch/loongarch.cc
> @@ -1643,6 +1643,13 @@ loongarch_classify_symbol (const_rtx x)
>         && !loongarch_symbol_binds_local_p (x))
>       return SYMBOL_GOT_DISP;
>   
> +  if (SYMBOL_REF_P (x))
> +    {
> +      tree decl = SYMBOL_REF_DECL (x);
> +      if (decl && lookup_attribute ("addr_global", DECL_ATTRIBUTES (decl)))
> +	return SYMBOL_GOT_DISP;
> +    }
> +
>     return SYMBOL_PCREL;
>   }
>   
> @@ -6068,6 +6075,54 @@ loongarch_starting_frame_offset (void)
>     return crtl->outgoing_args_size;
>   }
>   
> +static tree
> +loongarch_handle_addr_global_attribute (tree *node, tree name, tree, int,
> +					 bool *no_add_attrs)
> +{
> +  tree decl = *node;
> +  if (TREE_CODE (decl) == VAR_DECL)
> +    {
> +      if (DECL_CONTEXT (decl)
> +	  && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL
> +	  && !TREE_STATIC (decl))
> +	{
> +	  error_at (DECL_SOURCE_LOCATION (decl),
> +		    "%qE attribute cannot be specified for local "
> +		    "variables", name);
> +	  *no_add_attrs = true;
> +	}
> +    }
> +  else
> +    {
> +      warning (OPT_Wattributes, "%qE attribute ignored", name);
> +      *no_add_attrs = true;
> +    }
> +  return NULL_TREE;
> +}
> +
> +static const struct attribute_spec loongarch_attribute_table[] =
> +{
> +  /* { name, min_len, max_len, decl_req, type_req, fn_type_req,
> +       affects_type_identity, handler, exclude } */
> +  { "addr_global", 0, 0, true, false, false, false,
> +    loongarch_handle_addr_global_attribute, NULL },
> +  /* The last attribute spec is set to be NULL.  */
> +  {}
> +};
> +
> +bool
> +loongarch_use_anchors_for_symbol_p (const_rtx symbol)
> +{
> +  tree decl = SYMBOL_REF_DECL (symbol);
> +
> +  /* addr_global indicates we don't know how the linker will locate the symbol,
> +     so the use of anchor may cause relocation overflow.  */
> +  if (decl && lookup_attribute ("addr_global", DECL_ATTRIBUTES (decl)))
> +    return false;
> +
> +  return default_use_anchors_for_symbol_p (symbol);
> +}
> +
>   /* Initialize the GCC target structure.  */
>   #undef TARGET_ASM_ALIGNED_HI_OP
>   #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
> @@ -6256,6 +6311,12 @@ loongarch_starting_frame_offset (void)
>   #undef  TARGET_HAVE_SPECULATION_SAFE_VALUE
>   #define TARGET_HAVE_SPECULATION_SAFE_VALUE speculation_safe_value_not_needed
>   
> +#undef  TARGET_ATTRIBUTE_TABLE
> +#define TARGET_ATTRIBUTE_TABLE loongarch_attribute_table
> +
> +#undef  TARGET_USE_ANCHORS_FOR_SYMBOL_P
> +#define TARGET_USE_ANCHORS_FOR_SYMBOL_P loongarch_use_anchors_for_symbol_p
> +
>   struct gcc_target targetm = TARGET_INITIALIZER;
>   
>   #include "gt-loongarch.h"
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 7fe7f8817cd..4a1cd7ab5e4 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -7314,6 +7314,7 @@ attributes.
>   * Blackfin Variable Attributes::
>   * H8/300 Variable Attributes::
>   * IA-64 Variable Attributes::
> +* LoongArch Variable Attributes::
>   * M32R/D Variable Attributes::
>   * MeP Variable Attributes::
>   * Microsoft Windows Variable Attributes::
> @@ -8098,6 +8099,22 @@ defined by shared libraries.
>   
>   @end table
>   
> +@node LoongArch Variable Attributes
> +@subsection LoongArch Variable Attributes
> +
> +One attribute is currently defined for the LoongArch.
> +
> +@table @code
> +@item addr_global
> +@cindex @code{addr_global} variable attribute, LoongArch
> +@cindex variable addressability on the LoongArch
Can't understand this, Copied and forget to changing?
> +Use this attribute on the LoongArch to mark an object with address
> +unlimited limited by the local data section range specified by the code
> +model, even if the object is defined locally.  This attribute is mostly
> +useful if a @code{section} attribute or a linker script will move the
> +object somewhere unexpected by the code model.
> +@end table
> +
>   @node M32R/D Variable Attributes
>   @subsection M32R/D Variable Attributes
>   
> diff --git a/gcc/testsuite/gcc.target/loongarch/addr-global.c b/gcc/testsuite/gcc.target/loongarch/addr-global.c
> new file mode 100644
> index 00000000000..46a895c84bb
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/loongarch/addr-global.c
> @@ -0,0 +1,28 @@
> +/* addr_global attribute should mark x and y possibly outside of the local
> +   data range defined by the code model, so GOT should be used instead of
> +   PC-relative.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-mexplicit-relocs -mcmodel=normal -O2" } */
> +/* { dg-final { scan-assembler-not "%pc" } } */
> +/* { dg-final { scan-assembler-times "%got_pc_hi20" 3 } } */
> +
> +int x __attribute__((addr_global));
> +int y __attribute__((addr_global));
> +
> +int
> +test(void)
> +{
> +  return x + y;
> +}
> +
> +/* The following will be used for kernel per-cpu storage implemention. */
> +
> +register char *per_cpu_base __asm__("r21");
> +static int counter __attribute__((section(".data..percpu"), addr_global));
> +void
> +inc_counter(void)
> +{
> +  int *ptr = (int *)(per_cpu_base + (long)&counter);
> +  (*ptr)++;
> +}
  

Patch

diff --git a/gcc/config/loongarch/loongarch.cc b/gcc/config/loongarch/loongarch.cc
index 79687340dfd..db6f84d4e66 100644
--- a/gcc/config/loongarch/loongarch.cc
+++ b/gcc/config/loongarch/loongarch.cc
@@ -1643,6 +1643,13 @@  loongarch_classify_symbol (const_rtx x)
       && !loongarch_symbol_binds_local_p (x))
     return SYMBOL_GOT_DISP;
 
+  if (SYMBOL_REF_P (x))
+    {
+      tree decl = SYMBOL_REF_DECL (x);
+      if (decl && lookup_attribute ("addr_global", DECL_ATTRIBUTES (decl)))
+	return SYMBOL_GOT_DISP;
+    }
+
   return SYMBOL_PCREL;
 }
 
@@ -6068,6 +6075,54 @@  loongarch_starting_frame_offset (void)
   return crtl->outgoing_args_size;
 }
 
+static tree
+loongarch_handle_addr_global_attribute (tree *node, tree name, tree, int,
+					 bool *no_add_attrs)
+{
+  tree decl = *node;
+  if (TREE_CODE (decl) == VAR_DECL)
+    {
+      if (DECL_CONTEXT (decl)
+	  && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL
+	  && !TREE_STATIC (decl))
+	{
+	  error_at (DECL_SOURCE_LOCATION (decl),
+		    "%qE attribute cannot be specified for local "
+		    "variables", name);
+	  *no_add_attrs = true;
+	}
+    }
+  else
+    {
+      warning (OPT_Wattributes, "%qE attribute ignored", name);
+      *no_add_attrs = true;
+    }
+  return NULL_TREE;
+}
+
+static const struct attribute_spec loongarch_attribute_table[] =
+{
+  /* { name, min_len, max_len, decl_req, type_req, fn_type_req,
+       affects_type_identity, handler, exclude } */
+  { "addr_global", 0, 0, true, false, false, false,
+    loongarch_handle_addr_global_attribute, NULL },
+  /* The last attribute spec is set to be NULL.  */
+  {}
+};
+
+bool
+loongarch_use_anchors_for_symbol_p (const_rtx symbol)
+{
+  tree decl = SYMBOL_REF_DECL (symbol);
+
+  /* addr_global indicates we don't know how the linker will locate the symbol,
+     so the use of anchor may cause relocation overflow.  */
+  if (decl && lookup_attribute ("addr_global", DECL_ATTRIBUTES (decl)))
+    return false;
+
+  return default_use_anchors_for_symbol_p (symbol);
+}
+
 /* Initialize the GCC target structure.  */
 #undef TARGET_ASM_ALIGNED_HI_OP
 #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
@@ -6256,6 +6311,12 @@  loongarch_starting_frame_offset (void)
 #undef  TARGET_HAVE_SPECULATION_SAFE_VALUE
 #define TARGET_HAVE_SPECULATION_SAFE_VALUE speculation_safe_value_not_needed
 
+#undef  TARGET_ATTRIBUTE_TABLE
+#define TARGET_ATTRIBUTE_TABLE loongarch_attribute_table
+
+#undef  TARGET_USE_ANCHORS_FOR_SYMBOL_P
+#define TARGET_USE_ANCHORS_FOR_SYMBOL_P loongarch_use_anchors_for_symbol_p
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-loongarch.h"
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 7fe7f8817cd..4a1cd7ab5e4 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -7314,6 +7314,7 @@  attributes.
 * Blackfin Variable Attributes::
 * H8/300 Variable Attributes::
 * IA-64 Variable Attributes::
+* LoongArch Variable Attributes::
 * M32R/D Variable Attributes::
 * MeP Variable Attributes::
 * Microsoft Windows Variable Attributes::
@@ -8098,6 +8099,22 @@  defined by shared libraries.
 
 @end table
 
+@node LoongArch Variable Attributes
+@subsection LoongArch Variable Attributes
+
+One attribute is currently defined for the LoongArch.
+
+@table @code
+@item addr_global
+@cindex @code{addr_global} variable attribute, LoongArch
+@cindex variable addressability on the LoongArch
+Use this attribute on the LoongArch to mark an object with address
+unlimited limited by the local data section range specified by the code
+model, even if the object is defined locally.  This attribute is mostly
+useful if a @code{section} attribute or a linker script will move the
+object somewhere unexpected by the code model.
+@end table
+
 @node M32R/D Variable Attributes
 @subsection M32R/D Variable Attributes
 
diff --git a/gcc/testsuite/gcc.target/loongarch/addr-global.c b/gcc/testsuite/gcc.target/loongarch/addr-global.c
new file mode 100644
index 00000000000..46a895c84bb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/loongarch/addr-global.c
@@ -0,0 +1,28 @@ 
+/* addr_global attribute should mark x and y possibly outside of the local
+   data range defined by the code model, so GOT should be used instead of
+   PC-relative.  */
+
+/* { dg-do compile } */
+/* { dg-options "-mexplicit-relocs -mcmodel=normal -O2" } */
+/* { dg-final { scan-assembler-not "%pc" } } */
+/* { dg-final { scan-assembler-times "%got_pc_hi20" 3 } } */
+
+int x __attribute__((addr_global));
+int y __attribute__((addr_global));
+
+int
+test(void)
+{
+  return x + y;
+}
+
+/* The following will be used for kernel per-cpu storage implemention. */
+
+register char *per_cpu_base __asm__("r21");
+static int counter __attribute__((section(".data..percpu"), addr_global));
+void
+inc_counter(void)
+{
+  int *ptr = (int *)(per_cpu_base + (long)&counter);
+  (*ptr)++;
+}