diff mbox series

[committed] Revert 'Use more ARRAY_SIZE.' for mkoffload (was: [PATCH] Use more ARRAY_SIZE.)

Message ID ed8da6d6-8fa6-1e17-3238-5fe937cbb340@codesourcery.com
State New
Headers show
Series [committed] Revert 'Use more ARRAY_SIZE.' for mkoffload (was: [PATCH] Use more ARRAY_SIZE.) | expand

Commit Message

Tobias Burnus May 17, 2022, 7:03 p.m. UTC 
Hi Martin,

On 16.05.22 10:39, Martin Liška wrote:
> All right, CCing the following maintainers for other parts:
>> - David for JIT and Analyzer
>> - Tobias for Fortran part
>> - Jason for C-family part

Sorry for having missed that review request – and thanks to Mikael for
doing the review!

And thanks for the patch in general.

> There are not further comments from the remaining C-family part so I'm going
> to install the patch.

This patch broke offloading – fixed by reverting the patch for
{gcn,nvptx}/mkoffload.cc – and committed as obvious. Changing a C-code
generating string without telling the then called C compiler about the
macro won't fly.  See attachment for
r13-569-gc9852156dd2fedec130f6d8eb669579ef6237946, which reverts it for
mkoffload.cc, only.

Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

Comments

Martin Liška May 18, 2022, 7:40 a.m. UTC | #1 
On 5/17/22 21:03, Tobias Burnus wrote:
> This patch broke offloading – fixed by reverting the patch for
> {gcn,nvptx}/mkoffload.cc – and committed as obvious. Changing a C-code
> generating string without telling the then called C compiler about the
> macro won't fly.  See attachment for
> r13-569-gc9852156dd2fedec130f6d8eb669579ef6237946, which reverts it for
> mkoffload.cc, only.

Thank you for the fix and sorry for the code breakage ;)

Cheers,
Martin
diff mbox series

Patch

commit c9852156dd2fedec130f6d8eb669579ef6237946
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Tue May 17 20:46:29 2022 +0200

    Revert 'Use more ARRAY_SIZE.' for mkoffload
    
    Revert commit r13-472-gca32b29ec3e92dcf8dda5c2501d0baf9dd1cb09d partially;
    namely for {gcn,nvptx}/mkoffload.cc, only.
    
    The patch changed 'sizeof(...)/sizeof(...[0])' to the 'ARRAY_SIZE' macro,
    which is in principle a good idea – except that in the two mkoffload.cc,
    the change happened inside a string that is used to generate plain C code.
    
    With offlading to nvptx or gcn, the mkoffload genenates then the C file
    and compilation of the latter fails with
    "warning: implicit declaration of function 'ARRAY_SIZE'" followed by
    "error: initializer element is not constant"
    
    gcc/
            * config/gcn/mkoffload.cc (process_obj): Revert: Use ARRAY_SIZE.
            * config/nvptx/mkoffload.cc (process): Likewise.
---
 gcc/config/gcn/mkoffload.cc   | 2 +-
 gcc/config/nvptx/mkoffload.cc | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/gcc/config/gcn/mkoffload.cc b/gcc/config/gcn/mkoffload.cc
index 9db2124b129..94ba7ffa5af 100644
--- a/gcc/config/gcn/mkoffload.cc
+++ b/gcc/config/gcn/mkoffload.cc
@@ -723,7 +723,7 @@  process_obj (FILE *in, FILE *cfile)
 	   "  unsigned global_variable_count;\n"
 	   "} target_data = {\n"
 	   "  &gcn_image,\n"
-	   "  ARRAY_SIZE (gcn_kernels),\n"
+	   "  sizeof (gcn_kernels) / sizeof (gcn_kernels[0]),\n"
 	   "  gcn_kernels,\n"
 	   "  gcn_num_vars\n"
 	   "};\n\n");
diff --git a/gcc/config/nvptx/mkoffload.cc b/gcc/config/nvptx/mkoffload.cc
index fa3b4b76821..b28c1a32292 100644
--- a/gcc/config/nvptx/mkoffload.cc
+++ b/gcc/config/nvptx/mkoffload.cc
@@ -316,11 +316,11 @@  process (FILE *in, FILE *out)
 	   "  const struct nvptx_fn *fn_names;\n"
 	   "  unsigned fn_num;\n"
 	   "} target_data = {\n"
-	   "  ptx_objs, ARRAY_SIZE (ptx_objs),\n"
+	   "  ptx_objs, sizeof (ptx_objs) / sizeof (ptx_objs[0]),\n"
 	   "  var_mappings,"
-	   "  ARRAY_SIZE (var_mappings),\n"
+	   "  sizeof (var_mappings) / sizeof (var_mappings[0]),\n"
 	   "  func_mappings,"
-	   "  ARRAY_SIZE (func_mappings)\n"
+	   "  sizeof (func_mappings) / sizeof (func_mappings[0])\n"
 	   "};\n\n");
 
   fprintf (out, "#ifdef __cplusplus\n"