Skip to content
Commit 89493ee5 authored by Marc Herbert's avatar Marc Herbert Committed by Anas Nashif
Browse files

Revert "toolchain: gcc: Simplify `GEN_ABSOLUTE_SYM` and `GEN_ABSOLUTE_SYM_KCONFIG`"

This reverts commit 87779e73.

Commit 87779e73 ("toolchain: gcc: Simplify GEN_ABSOLUTE_SYM and
GEN_ABSOLUTE_SYM_KCONFIG") "unified the variants from using a
target-specific dialects to a target-agnostic solution". While reducing
duplication is always desired, this "unification" got rid of some
differences between architectures. Among others, this commit generalized
the use of the `c` constant operand modifier in the `GEN_ABSOLUTE_SYM()`
macro. Before this commit, the `c` modifier was NOT in use in a number
of architectures!

Generally speaking, reducing copy/paste/diverge must always be performed
in _two_ distinct phases with ample testing time between the two phases:

1. First, perform functional changes that remove the divergence.

   < give plenty enough time for testing >

2. Finally, remove the identical copies.

More specifically, I understand the `c` modifier may be required by some
architectures, but it's causing problems on other(s). Notably, it broke
the build with one pre-C11, gcc-based Xtensa toolchain:

```
14:03:17 kernel_offsets.h: Assembler messages:
14:03:17 kernel_offsets.h:28: Error: bad expression
14:03:17 kernel_offsets.h:28: Error: junk at end of line,
                                first unrecognized character is `c'
14:03:17 kernel_offsets.h:29: Error: bad expression
14:03:17 kernel_offsets.h:29: Error: junk at end of line,
                                first unrecognized character is `c'
...
```

While newer Xtensa toolchains accept `c`, they also ignore it. `c` makes
no difference whatsoever with all the (working) Xtensa toolchains I
tested. So for `Xtensa`, `c` is a best useless and at worse breaking the
build.

Up to gcc version 12 (2022), the `c` constant modifier was documented as
X86-specific!
https://gcc.gnu.org/onlinedocs/gcc-12.3.0/gcc/Extended-Asm.html#x86Operandmodifiers

Only starting with gcc version 13 (2023), the `c` modifier was
officially supported as "generic":
https://gcc.gnu.org/onlinedocs/gcc-13.1.0/gcc/Extended-Asm.html#Generic-Operand-Modifiers



`c` was very likely working with other architectures before 2023, but
not officially and we don't know which architectures and we don't know
when.

Note toolchain/gcc.h is included by toolchain/llvm.h and also used by
clang. The status of `c` across clang versions is unknown.

While I personally hate copy/paste/diverge with a passion, these macros
GEN_ABSOLUTE_SYM() and GEN_ABSOLUTE_SYM_KCONFIG() are very small so that
particular duplication is very reasonable. Architecture-specific code is
tricky and portability across toolchain and toolchain versions even
more. Testing low-level changes like this across different architectures
would also require demanding and unlikely coordination across different
architectures/companies.

Signed-off-by: default avatarMarc Herbert <marc.herbert@intel.com>
parent 51d80a98
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please to comment