[PATCH] optc-save-gen.awk: adjust generated array compare

[PATCH] optc-save-gen.awk: adjust generated array compare

[Chung-Lin Tang]

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

Hi Joseph,
Jan-Benedict reported a build-bot error for the nios2 port under --enable-werror-always:

options-save.cc: In function 'bool cl_target_option_eq(const cl_target_option*, const cl_target_option*)':
options-save.cc:9291:38: error: comparison between two arrays [-Werror=array-compare]
 9291 |   if (ptr1->saved_custom_code_status != ptr2->saved_custom_code_status
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
options-save.cc:9291:38: note: use unary '+' which decays operands to pointers or '&'component_ref' not supported by dump_decl<declaration error>[0] != &'component_ref' not supported by dump_decl<declaration error>[0]' to compare the addresses
options-save.cc:9294:37: error: comparison between two arrays [-Werror=array-compare]
 9294 |   if (ptr1->saved_custom_code_index != ptr2->saved_custom_code_index
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...

This is due to an array-typed TargetSave state in config/nios2/nios2.opt:
...
TargetSave
enum nios2_ccs_code saved_custom_code_status[256]

TargetSave
int saved_custom_code_index[256]
...


This patch adjusts the generated array state compare from 'ptr1->array' into '&ptr1->array[0]' in gcc/optc-save-gen.awk,
seems sufficient to pass the tougher checks.

Tested by ensuring the compiler builds, which should be sufficient here.
Okay to commit to mainline?

Thanks,
Chung-Lin

	* optc-save-gen.awk: Adjust array compare to use '&ptr->name[0]'
	instead of 'ptr->name'.

[-- Attachment #2: a.diff --]
[-- Type: text/plain, Size: 538 bytes --]

diff --git a/gcc/optc-save-gen.awk b/gcc/optc-save-gen.awk
index 233d1fbb637..27aabf2955e 100644
--- a/gcc/optc-save-gen.awk
+++ b/gcc/optc-save-gen.awk
@@ -1093,7 +1093,7 @@ for (i = 0; i < n_target_array; i++) {
 	name = var_target_array[i]
 	size = var_target_array_size[i]
 	type = var_target_array_type[i]
-	print "  if (ptr1->" name" != ptr2->" name "";
+	print "  if (&ptr1->" name"[0] != &ptr2->" name "[0]";
 	print "      || memcmp (ptr1->" name ", ptr2->" name ", " size " * sizeof(" type ")))"
 	print "    return false;";
 }

Re: [PATCH] optc-save-gen.awk: adjust generated array compare

[Jason Merrill]

On 9/8/22 11:29, Chung-Lin Tang wrote:
> Hi Joseph,
> Jan-Benedict reported a build-bot error for the nios2 port under --enable-werror-always:
> 
> options-save.cc: In function 'bool cl_target_option_eq(const cl_target_option*, const cl_target_option*)':
> options-save.cc:9291:38: error: comparison between two arrays [-Werror=array-compare]
>   9291 |   if (ptr1->saved_custom_code_status != ptr2->saved_custom_code_status
>        |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> options-save.cc:9291:38: note: use unary '+' which decays operands to pointers or '&'component_ref' not supported by dump_decl<declaration error>[0] != &'component_ref' not supported by dump_decl<declaration error>[0]' to compare the addresses
> options-save.cc:9294:37: error: comparison between two arrays [-Werror=array-compare]
>   9294 |   if (ptr1->saved_custom_code_index != ptr2->saved_custom_code_index
>        |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ...
> 
> This is due to an array-typed TargetSave state in config/nios2/nios2.opt:
> ...
> TargetSave
> enum nios2_ccs_code saved_custom_code_status[256]
> 
> TargetSave
> int saved_custom_code_index[256]
> ...
> 
> 
> This patch adjusts the generated array state compare from 'ptr1->array' into '&ptr1->array[0]' in gcc/optc-save-gen.awk,
> seems sufficient to pass the tougher checks.
> 
> Tested by ensuring the compiler builds, which should be sufficient here.
> Okay to commit to mainline?

Martin added the array support in r219636, any thoughts?

It seems to me that the warning is pointing out that comparing the 
address of the array is nonsensical, and we should remove it and just 
have the memcmp.

> Thanks,
> Chung-Lin
> 
> 	* optc-save-gen.awk: Adjust array compare to use '&ptr->name[0]'
> 	instead of 'ptr->name'.

Re: [PATCH] optc-save-gen.awk: adjust generated array compare

[Martin Liška]

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

On 9/8/22 18:23, Jason Merrill wrote:
> It seems to me that the warning is pointing out that comparing the address of the array is nonsensical, and we should remove it and just have the memcmp.

Yes, thanks for the pointer. We should always compare the array types with memcmp.

Ready to be installed?
Thanks,
Martin

[-- Attachment #2: 0001-opts-always-compare-array-option-values-with-memcmp.patch --]
[-- Type: text/x-patch, Size: 1002 bytes --]

From 32e875bf395ebb0444ced281c5e7634474100fb8 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Thu, 8 Sep 2022 20:00:33 +0200
Subject: [PATCH] opts: always compare array option values with memcmp

gcc/ChangeLog:

	* optc-save-gen.awk: Always compare array option values with memcmp.
---
 gcc/optc-save-gen.awk | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gcc/optc-save-gen.awk b/gcc/optc-save-gen.awk
index 233d1fbb637..49065ced0b3 100644
--- a/gcc/optc-save-gen.awk
+++ b/gcc/optc-save-gen.awk
@@ -1093,8 +1093,7 @@ for (i = 0; i < n_target_array; i++) {
 	name = var_target_array[i]
 	size = var_target_array_size[i]
 	type = var_target_array_type[i]
-	print "  if (ptr1->" name" != ptr2->" name "";
-	print "      || memcmp (ptr1->" name ", ptr2->" name ", " size " * sizeof(" type ")))"
+	print "  if (memcmp (ptr1->" name ", ptr2->" name ", " size " * sizeof(" type ")))"
 	print "    return false;";
 }
 for (i = 0; i < n_target_val; i++) {
-- 
2.37.3

Re: [PATCH] optc-save-gen.awk: adjust generated array compare

[Jason Merrill]

On 9/8/22 14:01, Martin Liška wrote:
> On 9/8/22 18:23, Jason Merrill wrote:
>> It seems to me that the warning is pointing out that comparing the address of the array is nonsensical, and we should remove it and just have the memcmp.
> 
> Yes, thanks for the pointer. We should always compare the array types with memcmp.
> 
> Ready to be installed?

OK.