diff mbox series

[v4,10/12] RISC-V/gas: warn about non-power-of-2 stack-align attribute

Message ID 6f5e4577-ec5e-4393-9df0-3b9c50de7d81@suse.com
State New
Headers
Series RISC-V: assorted fixes and (hopefully) improvements |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Jan Beulich July 24, 2026, 10:06 a.m. UTC 
  Only power-of-2 values are sensible for alignment.

Reviewed-by: Jiawei <jiawei@iscas.ac.cn>
---
Should we reject changing an already set non-zero value, like
riscv_merge_attributes() does?

Should we permit 0 as an explicit indication of "attribute not set"?
---
v2: New.

Comments

Nelson Chu July 27, 2026, 4:17 a.m. UTC | #1 
On Fri, Jul 24, 2026 at 6:06 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> Only power-of-2 values are sensible for alignment.
>
> Reviewed-by: Jiawei <jiawei@iscas.ac.cn>
> ---
> Should we reject changing an already set non-zero value, like
> riscv_merge_attributes() does?

The key lies in whether we care whether the previous elf attribute
settings still hold true.

* For elf architecture attributes,
Before assembling instructions: we don't care.
After assembling instructions: we care, but assuming the file-level
setting should represent the entire code, unless the function-level
settings intervene, we added the limitation to make sure it won't
change after assembling instructions.

* For priv-spec attributes
Before assembling instructions and csrs: we don't care.
After assembling instructions and csrs: we care.

* Otherwise
They shouldn't affect assembling any instructions, so we don't care.

Personally, for these non-arch and non-priv spec attributes, holding
true or not holding true for their previous settings are both right
and fine.  I don't have a strong opinion on this.

Thanks
Nelson

>
> Should we permit 0 as an explicit indication of "attribute not set"?

This probably needs to check what psABI defined what the 0 value
meant.  If they don't define, then I think we can treat 0 as "the
attribute not set".

> ---
> v2: New.
>
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -6137,6 +6137,13 @@ s_riscv_attribute (int ignored ATTRIBUTE
>                    "any instructions"));
>        break;
>
> +    case Tag_RISCV_stack_align:
> +      if (!attr[Tag_RISCV_stack_align].i
> +         || (attr[Tag_RISCV_stack_align].i &
> +             (attr[Tag_RISCV_stack_align].i - 1)))
> +       as_warn (_("`stack_align' attribute with non-power-of-2 value"));
> +      break;
> +
>      case Tag_RISCV_unaligned_access:
>        if (attr[Tag_RISCV_unaligned_access].i > 1)
>         as_warn (_("`unaligned_access' attribute with non-boolean value"));
> --- a/gas/testsuite/gas/riscv/attribute-warn.d
> +++ b/gas/testsuite/gas/riscv/attribute-warn.d
> @@ -4,4 +4,5 @@
>
>  Attribute Section: riscv
>  File Attributes
> +  Tag_RISCV_stack_align: 65536-bytes
>    Tag_RISCV_arch: [a-zA-Z0-9_\"].*
> --- a/gas/testsuite/gas/riscv/attribute-warn.l
> +++ b/gas/testsuite/gas/riscv/attribute-warn.l
> @@ -1,2 +1,4 @@
>  .*: Assembler messages:
>  .*:2: Warning: `unaligned_access' attribute with non-boolean value
> +.*:5: Warning: `stack_align' attribute with non-power-of-2 value
> +.*:6: Warning: `stack_align' attribute with non-power-of-2 value
> --- a/gas/testsuite/gas/riscv/attribute-warn.s
> +++ b/gas/testsuite/gas/riscv/attribute-warn.s
> @@ -1,3 +1,7 @@
>         .attribute unaligned_access, 1
>         .attribute unaligned_access, 0x1234
>         .attribute unaligned_access, 0
> +
> +       .attribute stack_align, 0
> +       .attribute stack_align, 0x4444
> +       .attribute stack_align, 0x10000
>
diff mbox series

Patch

--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -6137,6 +6137,13 @@  s_riscv_attribute (int ignored ATTRIBUTE
 		   "any instructions"));
       break;
 
+    case Tag_RISCV_stack_align:
+      if (!attr[Tag_RISCV_stack_align].i
+	  || (attr[Tag_RISCV_stack_align].i &
+	      (attr[Tag_RISCV_stack_align].i - 1)))
+	as_warn (_("`stack_align' attribute with non-power-of-2 value"));
+      break;
+
     case Tag_RISCV_unaligned_access:
       if (attr[Tag_RISCV_unaligned_access].i > 1)
 	as_warn (_("`unaligned_access' attribute with non-boolean value"));
--- a/gas/testsuite/gas/riscv/attribute-warn.d
+++ b/gas/testsuite/gas/riscv/attribute-warn.d
@@ -4,4 +4,5 @@ 
 
 Attribute Section: riscv
 File Attributes
+  Tag_RISCV_stack_align: 65536-bytes
   Tag_RISCV_arch: [a-zA-Z0-9_\"].*
--- a/gas/testsuite/gas/riscv/attribute-warn.l
+++ b/gas/testsuite/gas/riscv/attribute-warn.l
@@ -1,2 +1,4 @@ 
 .*: Assembler messages:
 .*:2: Warning: `unaligned_access' attribute with non-boolean value
+.*:5: Warning: `stack_align' attribute with non-power-of-2 value
+.*:6: Warning: `stack_align' attribute with non-power-of-2 value
--- a/gas/testsuite/gas/riscv/attribute-warn.s
+++ b/gas/testsuite/gas/riscv/attribute-warn.s
@@ -1,3 +1,7 @@ 
 	.attribute unaligned_access, 1
 	.attribute unaligned_access, 0x1234
 	.attribute unaligned_access, 0
+
+	.attribute stack_align, 0
+	.attribute stack_align, 0x4444
+	.attribute stack_align, 0x10000