[PATCH 0/2] target/hexagon: Fix macOS build

[PATCH 0/2] target/hexagon: Fix macOS build

[Anton Johansson via]

A default macOS build with xcode cli tools installed lacks the `indent`
program needed by the idef-parser postprocess step, remove this step. 

Additionally `cpp` used by the idef-parser preprocess step expands into
`clang ... -traditional-cpp` and doesn't support macro concatenation
among other things, replace it with a meson custom_target running
`${compiler} -E`.

fixes: https://lore.kernel.org/qemu-devel/d95ebf5d-c1f6-42c5-8aeb-65764fa87125@linaro.org/

Note: default bison on macOS is still too old (v2.3 vs v3.0) and a newer
version needs to be installed via homebrew. I'll take a look at
supporting v2.3.

Anton Johansson (2):
  target/hexagon: Replace `prepare` script with meson target
  target/hexagon: Drop `ident` postprocess step

 target/hexagon/idef-parser/prepare | 24 ------------------------
 target/hexagon/meson.build         | 24 ++++--------------------
 2 files changed, 4 insertions(+), 44 deletions(-)
 delete mode 100755 target/hexagon/idef-parser/prepare

-- 
2.47.1

[PATCH 1/2] target/hexagon: Replace `prepare` script with meson target

[Anton Johansson via]

The purpose of the prepare script is to invoke `cpp` to preprocess input
to idef-parser by expanding a few select macros.  On mac osx `cpp`
expands into `clang ... -traditional-cpp` which breaks macro
concatenation.  Replace `cpp` with `${compiler} -E`
and replace the script with a meson custom_target.

Signed-off-by: Anton Johansson <anjo@rev.ng>
---
 target/hexagon/idef-parser/prepare | 24 ------------------------
 target/hexagon/meson.build         |  3 ++-
 2 files changed, 2 insertions(+), 25 deletions(-)
 delete mode 100755 target/hexagon/idef-parser/prepare

diff --git a/target/hexagon/idef-parser/prepare b/target/hexagon/idef-parser/prepare
deleted file mode 100755
index cb3622d4f8..0000000000
--- a/target/hexagon/idef-parser/prepare
+++ /dev/null
@@ -1,24 +0,0 @@
-#!/usr/bin/env bash
-
-#
-#  Copyright(c) 2019-2021 rev.ng Labs Srl. All Rights Reserved.
-#
-#  This program is free software; you can redistribute it and/or modify
-#  it under the terms of the GNU General Public License as published by
-#  the Free Software Foundation; either version 2 of the License, or
-#  (at your option) any later version.
-#
-#  This program is distributed in the hope that it will be useful,
-#  but WITHOUT ANY WARRANTY; without even the implied warranty of
-#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-#  GNU General Public License for more details.
-#
-#  You should have received a copy of the GNU General Public License
-#  along with this program; if not, see <http://www.gnu.org/licenses/>.
-#
-
-set -e
-set -o pipefail
-
-# Run the preprocessor and drop comments
-cpp "$@"
diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build
index bb4ebaae81..abcf00ca1f 100644
--- a/target/hexagon/meson.build
+++ b/target/hexagon/meson.build
@@ -280,12 +280,13 @@ if idef_parser_enabled and 'hexagon-linux-user' in target_dirs
         command: [python, files('gen_idef_parser_funcs.py'), semantics_generated, '@OUTPUT@'],
     )
 
+    compiler = meson.get_compiler('c').get_id()
     preprocessed_idef_parser_input_generated = custom_target(
         'idef_parser_input.preprocessed.h.inc',
         output: 'idef_parser_input.preprocessed.h.inc',
         input: idef_parser_input_generated,
         depend_files: [idef_parser_dir / 'macros.h.inc'],
-        command: [idef_parser_dir / 'prepare', '@INPUT@', '-I' + idef_parser_dir, '-o', '@OUTPUT@'],
+        command: [compiler, '-x', 'c', '-E', '-I', idef_parser_dir, '-o', '@OUTPUT@', '@INPUT@'],
     )
 
     flex = generator(
-- 
2.47.1

[PATCH 2/2] target/hexagon: Drop `ident` postprocess step

[Anton Johansson via]

The indent command is not available on a default mac osx setup with
xcode cli tools installed.  While it does make idef-parser generated
code nicer to debug, it's not crucial and can be dropped.

Signed-off-by: Anton Johansson <anjo@rev.ng>
---
 target/hexagon/meson.build | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build
index abcf00ca1f..246dc7b241 100644
--- a/target/hexagon/meson.build
+++ b/target/hexagon/meson.build
@@ -323,30 +323,13 @@ if idef_parser_enabled and 'hexagon-linux-user' in target_dirs
         command: [idef_parser, '@INPUT@', '@OUTPUT0@', '@OUTPUT1@', '@OUTPUT2@']
     )
 
-    indent = find_program('indent', required: false)
-    if indent.found()
-        idef_generated_tcg_c = custom_target(
-            'indent',
-            input: idef_generated_tcg[0],
-            output: 'idef-generated-emitter.indented.c',
-            command: [indent, '-linux', '@INPUT@', '-o', '@OUTPUT@']
-        )
-    else
-        idef_generated_tcg_c = custom_target(
-            'copy',
-            input: idef_generated_tcg[0],
-            output: 'idef-generated-emitter.indented.c',
-            command: ['cp', '@INPUT@', '@OUTPUT@']
-        )
-    endif
-
     idef_generated_list = idef_generated_tcg[2].full_path()
 
-    hexagon_ss.add(idef_generated_tcg_c)
+    hexagon_ss.add(idef_generated_tcg[0])
 
     # Setup input and dependencies for the next step, this depends on whether or
     # not idef-parser is enabled
-    helper_dep = [semantics_generated, idef_generated_tcg_c, idef_generated_tcg]
+    helper_dep = [semantics_generated, idef_generated_tcg]
     helper_in = [semantics_generated, gen_tcg_h, gen_tcg_hvx_h, '--idef-parser', idef_generated_list]
 else
     # Setup input and dependencies for the next step, this depends on whether or
-- 
2.47.1

RE: [PATCH 2/2] target/hexagon: Drop `ident` postprocess step

[ltaylorsimpson]

> -----Original Message-----
> From: Anton Johansson <anjo@rev.ng>
> Sent: Wednesday, March 12, 2025 2:46 PM
> To: qemu-devel@nongnu.org
> Cc: ale@rev.ng; ltaylorsimpson@gmail.com; brian.cain@oss.qualcomm.com;
> philmd@linaro.org
> Subject: [PATCH 2/2] target/hexagon: Drop `ident` postprocess step
> 
> The indent command is not available on a default mac osx setup with xcode
> cli tools installed.  While it does make idef-parser generated code nicer
to
> debug, it's not crucial and can be dropped.
> 
> Signed-off-by: Anton Johansson <anjo@rev.ng>
> ---
>  target/hexagon/meson.build | 21 ++-------------------
>  1 file changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build
> index abcf00ca1f..246dc7b241 100644
> --- a/target/hexagon/meson.build
> +++ b/target/hexagon/meson.build
> @@ -323,30 +323,13 @@ if idef_parser_enabled and 'hexagon-linux-user' in
> target_dirs
>          command: [idef_parser, '@INPUT@', '@OUTPUT0@', '@OUTPUT1@',
> '@OUTPUT2@']
>      )
> 
> -    indent = find_program('indent', required: false)
> -    if indent.found()
> -        idef_generated_tcg_c = custom_target(
> -            'indent',
> -            input: idef_generated_tcg[0],
> -            output: 'idef-generated-emitter.indented.c',
> -            command: [indent, '-linux', '@INPUT@', '-o', '@OUTPUT@']
> -        )
> -    else
> -        idef_generated_tcg_c = custom_target(
> -            'copy',
> -            input: idef_generated_tcg[0],
> -            output: 'idef-generated-emitter.indented.c',
> -            command: ['cp', '@INPUT@', '@OUTPUT@']
> -        )
> -    endif
> -

I prefer to have the indented version present.

Is the above check/fallback not sufficient on MacOS?  It works on a Linux
system where indent is not present.

Thanks,
Taylor

Re: [PATCH 2/2] target/hexagon: Drop `ident` postprocess step

[Brian Cain]

On 3/24/2025 8:53 PM, ltaylorsimpson@gmail.com wrote:
>
>> -----Original Message-----
>> From: Anton Johansson <anjo@rev.ng>
>> Sent: Wednesday, March 12, 2025 2:46 PM
>> To: qemu-devel@nongnu.org
>> Cc: ale@rev.ng; ltaylorsimpson@gmail.com; brian.cain@oss.qualcomm.com;
>> philmd@linaro.org
>> Subject: [PATCH 2/2] target/hexagon: Drop `ident` postprocess step
>>
>> The indent command is not available on a default mac osx setup with xcode
>> cli tools installed.  While it does make idef-parser generated code nicer
> to
>> debug, it's not crucial and can be dropped.
>>
>> Signed-off-by: Anton Johansson <anjo@rev.ng>
>> ---
>>   target/hexagon/meson.build | 21 ++-------------------
>>   1 file changed, 2 insertions(+), 19 deletions(-)
>>
>> diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build
>> index abcf00ca1f..246dc7b241 100644
>> --- a/target/hexagon/meson.build
>> +++ b/target/hexagon/meson.build
>> @@ -323,30 +323,13 @@ if idef_parser_enabled and 'hexagon-linux-user' in
>> target_dirs
>>           command: [idef_parser, '@INPUT@', '@OUTPUT0@', '@OUTPUT1@',
>> '@OUTPUT2@']
>>       )
>>
>> -    indent = find_program('indent', required: false)
>> -    if indent.found()
>> -        idef_generated_tcg_c = custom_target(
>> -            'indent',
>> -            input: idef_generated_tcg[0],
>> -            output: 'idef-generated-emitter.indented.c',
>> -            command: [indent, '-linux', '@INPUT@', '-o', '@OUTPUT@']
>> -        )
>> -    else
>> -        idef_generated_tcg_c = custom_target(
>> -            'copy',
>> -            input: idef_generated_tcg[0],
>> -            output: 'idef-generated-emitter.indented.c',
>> -            command: ['cp', '@INPUT@', '@OUTPUT@']
>> -        )
>> -    endif
>> -
> I prefer to have the indented version present.
>
> Is the above check/fallback not sufficient on MacOS?  It works on a Linux
> system where indent is not present.


Aside: could using "clang-format" be another approach?  I suppose it's 
just exchanging one dependency for another, but maybe xcode comes w/this 
tool?  Then again, maybe it would be an inconvenient dependency on linux 
systems?



> Thanks,
> Taylor
>
>

Re: [PATCH 2/2] target/hexagon: Drop `ident` postprocess step

['Anton Johansson' via]

On 25/03/25, Brian Cain wrote:
> 
> On 3/24/2025 8:53 PM, ltaylorsimpson@gmail.com wrote:
> > 
> > > -----Original Message-----
> > > From: Anton Johansson <anjo@rev.ng>
> > > Sent: Wednesday, March 12, 2025 2:46 PM
> > > To: qemu-devel@nongnu.org
> > > Cc: ale@rev.ng; ltaylorsimpson@gmail.com; brian.cain@oss.qualcomm.com;
> > > philmd@linaro.org
> > > Subject: [PATCH 2/2] target/hexagon: Drop `ident` postprocess step
> > > 
> > > The indent command is not available on a default mac osx setup with xcode
> > > cli tools installed.  While it does make idef-parser generated code nicer
> > to
> > > debug, it's not crucial and can be dropped.
> > > 
> > > Signed-off-by: Anton Johansson <anjo@rev.ng>
> > > ---
> > >   target/hexagon/meson.build | 21 ++-------------------
> > >   1 file changed, 2 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build
> > > index abcf00ca1f..246dc7b241 100644
> > > --- a/target/hexagon/meson.build
> > > +++ b/target/hexagon/meson.build
> > > @@ -323,30 +323,13 @@ if idef_parser_enabled and 'hexagon-linux-user' in
> > > target_dirs
> > >           command: [idef_parser, '@INPUT@', '@OUTPUT0@', '@OUTPUT1@',
> > > '@OUTPUT2@']
> > >       )
> > > 
> > > -    indent = find_program('indent', required: false)
> > > -    if indent.found()
> > > -        idef_generated_tcg_c = custom_target(
> > > -            'indent',
> > > -            input: idef_generated_tcg[0],
> > > -            output: 'idef-generated-emitter.indented.c',
> > > -            command: [indent, '-linux', '@INPUT@', '-o', '@OUTPUT@']
> > > -        )
> > > -    else
> > > -        idef_generated_tcg_c = custom_target(
> > > -            'copy',
> > > -            input: idef_generated_tcg[0],
> > > -            output: 'idef-generated-emitter.indented.c',
> > > -            command: ['cp', '@INPUT@', '@OUTPUT@']
> > > -        )
> > > -    endif
> > > -
> > I prefer to have the indented version present.
> > 
> > Is the above check/fallback not sufficient on MacOS?  It works on a Linux
> > system where indent is not present.
> 
> 
> Aside: could using "clang-format" be another approach?  I suppose it's just
> exchanging one dependency for another, but maybe xcode comes w/this tool? 
> Then again, maybe it would be an inconvenient dependency on linux systems?
> 
> 
> 
> > Thanks,
> > Taylor

Sorry, I misspoke in the commit message. The problem does not occur for a
default xcode commandline tools install, but when the user has installed
indent via homebrew or some other source. I don't have access to a mac
at this moment so I cannot verify, but I think `-linux` is not
supported.

If we want to handle this situation we can either expand `-linux` into
it's constituent flags or add a `host_os == 'linux'` condition.

Re: [PATCH 2/2] target/hexagon: Drop `ident` postprocess step

[Brian Cain]

On 3/26/2025 7:42 AM, 'Anton Johansson' wrote:
> On 25/03/25, Brian Cain wrote:
>> On 3/24/2025 8:53 PM, ltaylorsimpson@gmail.com wrote:
>>>> -----Original Message-----
>>>> From: Anton Johansson <anjo@rev.ng>
>>>> Sent: Wednesday, March 12, 2025 2:46 PM
>>>> To: qemu-devel@nongnu.org
>>>> Cc: ale@rev.ng; ltaylorsimpson@gmail.com; brian.cain@oss.qualcomm.com;
>>>> philmd@linaro.org
>>>> Subject: [PATCH 2/2] target/hexagon: Drop `ident` postprocess step
>>>>
>>>> The indent command is not available on a default mac osx setup with xcode
>>>> cli tools installed.  While it does make idef-parser generated code nicer
>>> to
>>>> debug, it's not crucial and can be dropped.
>>>>
>>>> Signed-off-by: Anton Johansson <anjo@rev.ng>
>>>> ---
>>>>    target/hexagon/meson.build | 21 ++-------------------
>>>>    1 file changed, 2 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build
>>>> index abcf00ca1f..246dc7b241 100644
>>>> --- a/target/hexagon/meson.build
>>>> +++ b/target/hexagon/meson.build
>>>> @@ -323,30 +323,13 @@ if idef_parser_enabled and 'hexagon-linux-user' in
>>>> target_dirs
>>>>            command: [idef_parser, '@INPUT@', '@OUTPUT0@', '@OUTPUT1@',
>>>> '@OUTPUT2@']
>>>>        )
>>>>
>>>> -    indent = find_program('indent', required: false)
>>>> -    if indent.found()
>>>> -        idef_generated_tcg_c = custom_target(
>>>> -            'indent',
>>>> -            input: idef_generated_tcg[0],
>>>> -            output: 'idef-generated-emitter.indented.c',
>>>> -            command: [indent, '-linux', '@INPUT@', '-o', '@OUTPUT@']
>>>> -        )
>>>> -    else
>>>> -        idef_generated_tcg_c = custom_target(
>>>> -            'copy',
>>>> -            input: idef_generated_tcg[0],
>>>> -            output: 'idef-generated-emitter.indented.c',
>>>> -            command: ['cp', '@INPUT@', '@OUTPUT@']
>>>> -        )
>>>> -    endif
>>>> -
>>> I prefer to have the indented version present.
>>>
>>> Is the above check/fallback not sufficient on MacOS?  It works on a Linux
>>> system where indent is not present.
>>
>> Aside: could using "clang-format" be another approach?  I suppose it's just
>> exchanging one dependency for another, but maybe xcode comes w/this tool?
>> Then again, maybe it would be an inconvenient dependency on linux systems?
>>
>>
>>
>>> Thanks,
>>> Taylor
> Sorry, I misspoke in the commit message. The problem does not occur for a
> default xcode commandline tools install, but when the user has installed
> indent via homebrew or some other source. I don't have access to a mac
> at this moment so I cannot verify, but I think `-linux` is not
> supported.
>
> If we want to handle this situation we can either expand `-linux` into
> it's constituent flags or add a `host_os == 'linux'` condition.


I wouldn't bind this behavior to the hexagon-linux-user target, because 
it happens to be hosted on "!macos" if that's what you mean.  But 
"host_os == linux" seems reasonable.


OTOH if it's not easy to detect this condition, I think it's fine to 
remove it entirely.  Seems like -- if we as developers want to read the 
output we can indent it on demand (or use something like inotifywait + 
indent to automagically indent it after writing).


Nit: subject typo - s/ident/indent/