[SKIP CI] sparse: temporarily ignore MTL address errors and add to daily tests #6722
Conversation
marc-hb
force-pushed
the
sparse-mtl-daily
branch
from
e2622b7 to
7b96338
Compare
|
MTL warnings are still visible in https://github.com/thesofproject/sof/actions/runs/3625503925/jobs/6113619508 but they don't "break the build" any more. EDIT: same again in https://github.com/thesofproject/sof/actions/runs/3688703910/jobs/6243807588 https://github.com/thesofproject/sof/actions/runs/3625503919/jobs/6113620156 has the stupid "we don't talk about sparse" checkpatch warning and nothing else. |
marc-hb
requested review from
aborisovich,
cujomalainey,
dbaluta and
lgirdwood
as code owners
marc-hb
requested review from
abonislawski,
aiChaoSONG,
andyross,
greg-intel,
juimonen,
keqiaozhang,
kv2019i,
lyakh,
marcinszkudlinski and
ranj063
scripts/parse_sparse_output.sh
Outdated
There was a problem hiding this comment.
this means, we will see complete MTL sparse output including "address space" warnings in CI, it just won't be a failure? Could you maybe remove "SKIP CI" to let us see what it will look like?
There was a problem hiding this comment.
this means, we will see complete MTL sparse output including "address space" warnings in CI, it just won't be a failure?
Yes, see link in comment above and in the "Checks" box below.
Could you maybe remove "SKIP CI"
Only sof-ci/jenkins reads "SKIP CI", Github does not care. Again look at the "Details" box below.
There was a problem hiding this comment.
Indeed, thanks! I'm not sure whether it's a good idea to green-wash our failures, but as long as we have the output, I personally can live with it
There was a problem hiding this comment.
I don't like it either but it's been red for too long now. I also made sure it's very easy to revert.
Can you approve then?
There was a problem hiding this comment.
MTL errors will be fixed after v2.4 is tagged - lets fix instead of hiding. The other patches are fine in this PR.
There was a problem hiding this comment.
@lgirdwood re-submitting the other patches is not an issue but does anyone have any idea how to fix the current warnings on MTL and a line of sight on a fix? Right now we are actively training users to ignore the results on MTL and we are blind to possibly WORSE MTL regressions.
There was a problem hiding this comment.
Will be fixed after v2.4 is tagged, IOW early next week.
scripts/parse_sparse_output.sh
Outdated
There was a problem hiding this comment.
MTL errors will be fixed after v2.4 is tagged - lets fix instead of hiding. The other patches are fine in this PR.
|
I resubmitted the cleanup commits that don't change anything in this new PR:
I am sure it's a bad idea to "red-wash" our sparse warnings on MTL, which is what we have been doing for a few weeks now. In other words: if the sparse analysis degrades from "we have warnings" to "it does not even build" then no one will notice. "Red-washed". This of course on top of training people to ignore MTL warnings in the first place. |
We cannot keep CI checks failing for that long. Should be reverted as soon as the code is fixed. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Because why not. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
marc-hb
force-pushed
the
sparse-mtl-daily
branch
from
7b96338 to
44813d1
Compare
|
@lyakh how are the SPARSE MTL fixes in Zephyr coming ? |
|
I believe all sparse warnings are fixed in the Zephyr main branch but upgrading Zephyr is blocked by zephyrproject-rtos/zephyr#53327 (comment) |
|
Actually let's keep this for adding to daily tests |
|
address space warnings are all fixed now. Adding sparse to daily tests in new |
There was an address space regression and no one noticed: |
3 participants
6 commits, see messages