Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SKIP CI] parse_sparse_output.sh: add "removes address space" and "too many" #7488

Merged
merged 1 commit into from
Apr 25, 2023

Conversation

marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Apr 21, 2023

Catch the additional two warnings below:

warning: too many warnings
warning: cast removes address space

@marc-hb marc-hb requested a review from lyakh April 21, 2023 05:39
@marc-hb
Copy link
Collaborator Author

marc-hb commented Apr 21, 2023

This is unhiding the following warnings:
https://github.com/thesofproject/sof/actions/runs/4761818377/jobs/8463408808?pr=7488

/zep_workspace/zephyr/drivers/dai/intel/ssp/ssp.c:2084:17: warning: too many warnings
/zep_workspace/sof/zephyr/../src/include/sof/audio/component.h:826:9: warning: too many warnings
/zep_workspace/sof/src/audio/src/src.c:977:21: warning: cast removes address space '__cache' of expression
/zep_workspace/sof/src/audio/src/src.c:977:18: warning: incorrect type in assignment (different address spaces)
/zep_workspace/sof/src/audio/src/src.c:978:19: warning: cast removes address space '__cache' of expression
/zep_workspace/sof/src/audio/src/src.c:978:16: warning: incorrect type in assignment (different address spaces)
/zep_workspace/sof/src/lib/ams.c:61:29: warning: cast removes address space '__cache' of expression
/zep_workspace/sof/src/lib/ams.c:68:44: warning: cast removes address space '__cache' of expression
/zep_workspace/sof/src/lib/ams.c:288:41: warning: cast removes address space '__cache' of expression

EDIT: src. fixes submitted in #7492

Copy link
Collaborator

@paulstelian97 paulstelian97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth having enabled. That said are those warnings useful on current platforms, or just good for the purpose of porting to new ones?

@marc-hb
Copy link
Collaborator Author

marc-hb commented Apr 21, 2023

According to @lyakh (please correct me) these two warnings should have always been caught from the beginning, this was a miss. This is not checking something new.

@marc-hb

This comment was marked as outdated.

@marc-hb marc-hb changed the title [SKIP CI] parse_sparse_output.sh: add "removes address space" and "too many" parse_sparse_output.sh: add "removes address space" and "too many" Apr 21, 2023
@marc-hb
Copy link
Collaborator Author

marc-hb commented Apr 21, 2023

After #7492 was merged the list got smaller in https://github.com/thesofproject/sof/actions/runs/4769307682/jobs/8479546182?pr=7488

/zep_workspace/zephyr/drivers/dai/intel/ssp/ssp.c:2084:17: warning: too many warnings
/zep_workspace/sof/zephyr/../src/include/sof/audio/component.h:826:9: warning: too many warnings
/zep_workspace/sof/src/lib/ams.c:61:29: warning: cast removes address space '__cache' of expression
/zep_workspace/sof/src/lib/ams.c:68:44: warning: cast removes address space '__cache' of expression
/zep_workspace/sof/src/lib/ams.c:288:41: warning: cast removes address space '__cache' of expression

EDIT: --cmake-args=-DCONFIG_AMS=n gets rid of the ams.c warnings. Only for sparse, in the short term? @lyakh ?

@lyakh
Copy link
Collaborator

lyakh commented Apr 24, 2023

EDIT: --cmake-args=-DCONFIG_AMS=n gets rid of the ams.c warnings. Only for sparse, in the short term? @lyakh ?

I'd rather fix AMS - IIUC it's supposed to be a rather central common functionality, included by default in most builds

@lyakh
Copy link
Collaborator

lyakh commented Apr 24, 2023

/zep_workspace/zephyr/drivers/dai/intel/ssp/ssp.c:2084:17: warning: too many warnings
/zep_workspace/sof/zephyr/../src/include/sof/audio/component.h:826:9: warning: too many warnings

@marc-hb I don't see these warnings in the logs, that you've linked?
UPDATE: found them in https://github.com/thesofproject/sof/actions/runs/4769307682/jobs/8479546253?pr=7488 - one of them should be fixed by zephyrproject-rtos/zephyr#56972 as for the one in component.h - I see where the warning comes from, and we could silence it by making the data array in struct sof_abi_hdr of size 0 instead of flexible, as we've done in multiple other locations. Still I'm not sure how that single warning gets multiplied to "too many"

@marc-hb
Copy link
Collaborator Author

marc-hb commented Apr 24, 2023

@marc-hb I don't see these warnings in the logs, that you've linked?

They were probably in MTL while I linked to TGL or vice-versa.

@marc-hb marc-hb marked this pull request as draft April 24, 2023 15:28
@marc-hb marc-hb changed the title parse_sparse_output.sh: add "removes address space" and "too many" [SKIP CI] parse_sparse_output.sh: add "removes address space" and "too many" Apr 24, 2023
@marc-hb
Copy link
Collaborator Author

marc-hb commented Apr 24, 2023

Only the 3 AMS warnings left in https://github.com/thesofproject/sof/actions/runs/4789322954/jobs/8517045526?pr=7488 after the big zephyr upgrade #6480! \o/

EDIT: AMS just fixed in #7501

Catch the additional two warnings below:

warning: too many warnings
warning: cast removes address space

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb marc-hb marked this pull request as ready for review April 24, 2023 22:01
@marc-hb
Copy link
Collaborator Author

marc-hb commented Apr 24, 2023

Green sparse log in https://github.com/thesofproject/sof/actions/runs/4791424286/jobs/8521820464?pr=7488

Thanks @lyakh !

Can we merge this quickly before someone adds a new sparse regression? :-)

Ooops: re-requested @lyakh 's review and lost his approval by accident. It's still above on April 23.

https://sof-ci.01.org/sof-pr-viewer/#/build/PR7488/build11882024 is red but this PR is a pure SPARSE filter change, totally unrelated.

@marc-hb marc-hb requested review from lyakh and lgirdwood April 24, 2023 22:03
@kv2019i
Copy link
Collaborator

kv2019i commented Apr 25, 2023

The CI failures are unrelated as this PR only changes the sparse script, proceeding with merge.

@kv2019i kv2019i merged commit d345c56 into thesofproject:main Apr 25, 2023
@marc-hb marc-hb deleted the sparse-too-many branch April 26, 2023 05:15
@marc-hb
Copy link
Collaborator Author

marc-hb commented Apr 26, 2023

as for the one in component.h - I see where the warning comes from, and we could silence it by making the data array in struct sof_abi_hdr of size 0 instead of flexible, as we've done in multiple other locations.

Actually the warning in component.h is on this (the comment rang a bell, then git blame pointed it at... me)

	/* Flexible array members[] are forbidden in unions but this
	 * does not seem to bother gcc as long as non-standard
	 * zero-length arrays[0] are used instead.  Nesting flexible
	 * array member declarations in arrays or structures is
	 * forbidden too. Cleaning this up would likely require code
	 * changes to explicitly cast intermediate steps.
	 */
	/* control data - add new types if needed */
	union {
		/* channel values can be used by volume type controls */
		struct sof_ipc_ctrl_value_chan chanv[0];
		/* component values used by routing controls like mux, mixer */
		struct sof_ipc_ctrl_value_comp compv[0];
		/* data can be used by binary controls */
		struct sof_abi_hdr data[0];
	};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants