Fixes for i.MX xt-clang build #7538

paulstelian97 · 2023-04-28T10:16:58Z

These two commits, together with upstream patch zephyrproject-rtos/zephyr#57374 fix the build of SOF for i.MX platforms with the new xt-clang Cadence 2023 toolchain.

Keeping as draft until the Zephyr patch is merged. (edit: Zephyr patch merged, west manifest updated)

marc-hb · 2023-04-28T14:27:38Z

src/arch/xtensa/include/xtensa/config/core.h

+
+#ifndef UINT32_C
+#define UINT32_C(x) __UINT32_C(x)
+#endif


This #ifndef #define pattern is at best a necessary evil because the definition can change depending the #include order (recent example: 2512698) and the direction the wind blows.

So can you transfer some of the information and justification from the commit message to the comment here?

The reason I had to put it in here in the first place, especially with the __ZEPHYR__ guard, is because of the include order. On XTOS builds (including i.MX8) without that guard this is included before newlib and newlib does not do this, which leads to conflicts.

I will copy the comment and resubmit, maybe today but if not on Tuesday. (edit: done)

On XTOS builds (including i.MX8) without that guard this is included before newlib and newlib does not do this, which leads to conflicts.

What sort of conflicts? This is coming up again in non-Zephyr build #9351 (comment) ...

Maybe @keith-packard or @stephanosio could help us get out of this libc / toolchain nightmare?

dbaluta · 2023-05-02T16:16:24Z

Now because Zephyr PR was merged I think we are ready to get this out of draft state.

@iuliana-prodan should we update the west yaml to pull in the new zephyr code?

iuliana-prodan · 2023-05-04T08:02:44Z

Now because Zephyr PR was merged I think we are ready to get this out of draft state.

@iuliana-prodan should we update the west yaml to pull in the new zephyr code?

Yes, definitely we should update the west.yaml with latest zephyr (to bring the changes needed for i.MX with xt-clang).
@paulstelian97 I agree with @dbaluta to add a commit for updating west.yaml in this PR.

dbaluta · 2023-05-04T12:16:15Z

@lgirdwood :

are we using https://github.com/zephyrproject-rtos/zephyr/ with sof? or https://github.com/thesofproject/zephyr
should we create a separate PR to upgrade the zephyr hash?

kv2019i · 2023-05-04T12:41:42Z

@dbaluta wrote:

* are we using https://github.com/zephyrproject-rtos/zephyr/ with sof? or https://github.com/thesofproject/zephyr
* should we create a separate PR to upgrade the zephyr hash?

We are using https://github.com/zephyrproject-rtos/zephyr/ and the version defined in sof/west.yml.

So you can add a commit to this PR to update the Zephyr version (see git log of sof/west.yml for examples). If we have a breaking interface change (like the SYS_INIT last week), then the SOF change and west.yml update need to be squashed into a single git commit, in order not to break git bisect.

dbaluta · 2023-05-04T12:44:15Z

@lyakh great! then we are fine. A commit to update zephyr has was already added to this PR. Also, how are you doing for stable-v2.x version?

kv2019i · 2023-05-04T15:06:39Z

@dbaluta wrote:

@lyakh great! then we are fine. A commit to update zephyr has was already added to this PR. Also, how are you doing for stable-v2.x version?

stable-v2.x branches have their own west.yml and there we've either used upstream Zephyr repo (if no backports are needed to Zephyr), or SOF zephyr clone (if Zephyr backports are needed for the stable series).

The west.yml specifies both the repo and commit, so we can easily switch. Any any SOF branch/commit will always have a clear mapping to a Zephyr version, so there's no ambiguity on the version used.

PS west.yml can also point to an unmerged pull request. This is a nice tool to test a Zephyr change with SOF CI (before it gets merged).

marc-hb · 2023-05-05T03:56:41Z

are we using https://github.com/zephyrproject-rtos/zephyr/ [upstream] with sof? or https://github.com/thesofproject/zephyr [fork]

Short answer: upstream every time it's possible, fork when it's not.

Long answer: see @kv2019i's above :-)

kv2019i

Looks good, let's see the CI for the Zephyr update (should be good).

kv2019i · 2023-05-11T08:45:49Z

@tmleman Do we need #7174 to make the INtel MTL test pass, or is this something else (this PR moves to a newer Zephyr).

tmleman · 2023-05-11T09:03:26Z

Do we need #7174 to make the INtel MTL test pass, or is this something else (this PR moves to a newer Zephyr).

@kv2019i If we do a zephyr update without my patch, the secondary cores will stop working on MTL. Currently, there are probably no tests for this in CI, so the fail's are rather unrelated.

paulstelian97 · 2023-05-11T09:09:07Z

I don't have any qualms with which specific version of Zephyr to update to, my pull request just holds the earliest commit that solves the problem, but I wouldn't mind anything newer (or an update on the main branch that I can rebase on top of, which is even better IMO)

dbaluta · 2023-05-11T10:38:41Z

@tmleman so should we merge #7174 first? Or should we merge current PR now?

Is #7174 ready? We would like to have this patch merged asap because we have an internal release coming.

tmleman · 2023-05-11T15:39:08Z

@tmleman so should we merge #7174 first? Or should we merge current PR now?

Is #7174 ready? We would like to have this patch merged asap because we have an internal release coming.

@dbaluta #7174 is ready to be merged and I think it would be better if it gets merged first. It would make bisecting easier in case of any problems.

kv2019i · 2023-05-11T16:05:23Z

~~@wszypelt Ok to proceed, this is not touching Intel platforms.?~~ short term memory loss on my part, sorry for the noise. so now I read my own comment from today and will proceed and look at #7174

kv2019i · 2023-05-11T16:09:06Z

@paulstelian97 #7174 merged, can you rebase and let's see if the CI passes now.

This define is used by the new 2023 xt-clang toolchain and, while there are a few definitions (identical to this one) in various implementations such as newlib, none of them is in use when building SOF with Zephyr and XtensaTools. Add this define so that the toolchain provided headers work correctly. Signed-off-by: Paul Olaru <paul.olaru@nxp.com>

For the functions platform_interrupt_clear and platform_interrupt_set, the existing definitions based on arch_interrupt_* do not compile with the xt-clang 2023 toolchain for imx. Use the Zephyr wrapper implementations for those for now. Signed-off-by: Paul Olaru <paul.olaru@nxp.com>

paulstelian97 · 2023-05-12T06:58:29Z

Main branch still behind the commit I need, do I keep the update? I pushed without, can push the extra commit if necessary.

This update is needed to include Zephyr specific patches required for building SOF on i.MX platforms with Xtensa toolchain. Signed-off-by: Paul Olaru <paul.olaru@nxp.com>

dbaluta · 2023-05-12T07:04:57Z

@paulstelian97 we still need to zephyr update, right? otherwise, the build will fail for SOF imx8 with xt-clang.

lyakh · 2023-05-12T08:39:37Z

src/arch/xtensa/include/xtensa/config/core.h

+#define UINT32_C(x) __UINT32_C(x)
+#endif
+
+#endif


is this really a suitable header for this? This seems some compiler compatibility support, how about xtos/include/sof/compiler_attributes.h?

I'm pretty sure the xtos/ subfolder isn't used in Zephyr builds; I want the opposite (something that only applies to Zephyr)

paulstelian97 force-pushed the xt-clang-updates branch from a642a4d to 4061b5a Compare April 28, 2023 10:55

marc-hb reviewed Apr 28, 2023

View reviewed changes

paulstelian97 force-pushed the xt-clang-updates branch from 4061b5a to f355573 Compare April 28, 2023 14:55

paulstelian97 marked this pull request as ready for review May 4, 2023 06:09

paulstelian97 requested review from kv2019i, lyakh, iuliana-prodan, dbaluta, abonislawski, lgirdwood, plbossart, mmaka1 and lbetlej as code owners May 4, 2023 06:09

paulstelian97 force-pushed the xt-clang-updates branch from bf85fab to 6a4e769 Compare May 4, 2023 10:34

dbaluta approved these changes May 4, 2023

View reviewed changes

kv2019i approved these changes May 11, 2023

View reviewed changes

Paul Olaru added 2 commits May 12, 2023 09:56

west.yml: Upgrade zephyr to fa5117225a

bf137f3

This update is needed to include Zephyr specific patches required for building SOF on i.MX platforms with Xtensa toolchain. Signed-off-by: Paul Olaru <paul.olaru@nxp.com>

paulstelian97 force-pushed the xt-clang-updates branch from 6a4e769 to 480b24e Compare May 12, 2023 06:59

lyakh reviewed May 12, 2023

View reviewed changes

dbaluta merged commit 1acead1 into thesofproject:main May 15, 2023

paulstelian97 deleted the xt-clang-updates branch May 17, 2023 08:26

This was referenced Aug 27, 2024

Add the new platform ACP_7_0 #9351

Merged

Include xtensa-types.h header file to support the ACP_7_0 platform's build. #9413

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes for i.MX xt-clang build #7538

Fixes for i.MX xt-clang build #7538

paulstelian97 commented Apr 28, 2023 •

edited

Loading

marc-hb Apr 28, 2023

paulstelian97 Apr 28, 2023 •

edited by marc-hb

Loading

marc-hb Aug 27, 2024

dbaluta commented May 2, 2023

iuliana-prodan commented May 4, 2023

dbaluta commented May 4, 2023

kv2019i commented May 4, 2023

dbaluta commented May 4, 2023 •

edited

Loading

kv2019i commented May 4, 2023

marc-hb commented May 5, 2023

kv2019i left a comment

kv2019i commented May 11, 2023

tmleman commented May 11, 2023

paulstelian97 commented May 11, 2023

dbaluta commented May 11, 2023

tmleman commented May 11, 2023

kv2019i commented May 11, 2023 •

edited

Loading

kv2019i commented May 11, 2023

paulstelian97 commented May 12, 2023 •

edited

Loading

dbaluta commented May 12, 2023

lyakh May 12, 2023

paulstelian97 May 12, 2023

Fixes for i.MX xt-clang build #7538

Fixes for i.MX xt-clang build #7538

Conversation

paulstelian97 commented Apr 28, 2023 • edited Loading

marc-hb Apr 28, 2023

Choose a reason for hiding this comment

paulstelian97 Apr 28, 2023 • edited by marc-hb Loading

Choose a reason for hiding this comment

marc-hb Aug 27, 2024

Choose a reason for hiding this comment

dbaluta commented May 2, 2023

iuliana-prodan commented May 4, 2023

dbaluta commented May 4, 2023

kv2019i commented May 4, 2023

dbaluta commented May 4, 2023 • edited Loading

kv2019i commented May 4, 2023

marc-hb commented May 5, 2023

kv2019i left a comment

Choose a reason for hiding this comment

kv2019i commented May 11, 2023

tmleman commented May 11, 2023

paulstelian97 commented May 11, 2023

dbaluta commented May 11, 2023

tmleman commented May 11, 2023

kv2019i commented May 11, 2023 • edited Loading

kv2019i commented May 11, 2023

paulstelian97 commented May 12, 2023 • edited Loading

dbaluta commented May 12, 2023

lyakh May 12, 2023

Choose a reason for hiding this comment

paulstelian97 May 12, 2023

Choose a reason for hiding this comment

paulstelian97 commented Apr 28, 2023 •

edited

Loading

paulstelian97 Apr 28, 2023 •

edited by marc-hb

Loading

dbaluta commented May 4, 2023 •

edited

Loading

kv2019i commented May 11, 2023 •

edited

Loading

paulstelian97 commented May 12, 2023 •

edited

Loading