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

Backport ci changes #21754

Merged
merged 9 commits into from
Mar 7, 2024
Merged

Conversation

pamaury
Copy link
Contributor

@pamaury pamaury commented Feb 29, 2024

This is a backport of #21580, #21645 and #20836 but I had to fix every single commit with conflicts so please double-check carefully @nbdd0121 and @jwnrt

I also re-enabled the rom and rom_test jobs but added a condition so that they only run on the master branch. The idea is that we can back-backport this change to master and avoid divergences between the azure files (thanks @nbdd0121 for the suggestion)

azure-pipelines.yml Outdated Show resolved Hide resolved
@pamaury pamaury force-pushed the backport_ci_changes branch from 1b70af9 to 8a98fca Compare February 29, 2024 13:28
@pamaury
Copy link
Contributor Author

pamaury commented Feb 29, 2024

Just to help review: with this change, the only diff between master and earlgray_es_sival for the azure pipelines are:

  • condition on rom and rom_ext dependent on branch -> will be backported to master
  • branch and bucket for GCP -> different names on master and earlgrey_es_sival so that's fine
  • changes introduced by #[tracking] Use ipgen to produce templated IPs #8440 related to ip_gen (probably not needed for es_sival branch since RTL does not change?)

Copy link
Contributor

@jwnrt jwnrt left a comment

Choose a reason for hiding this comment

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

The changes here LGTM, good idea to conditionally re-enable those ROM FPGA tests. I'll have another look at the logs of the CI run once it's finished to make sure it's doing what we expect.

@pamaury pamaury force-pushed the backport_ci_changes branch from 7ab9ed4 to 489c33c Compare March 4, 2024 11:52
@pamaury
Copy link
Contributor Author

pamaury commented Mar 4, 2024

I had to do so more manual fixing because we have a couple more tests in es_sival and those still had --bitstream. I also early-backported part of #21757 to remove manual-tagged tests to avoid running pmod tests.

@nbdd0121
Copy link
Contributor

nbdd0121 commented Mar 5, 2024

The log says the ROM job is doing bitstream build now

@pamaury pamaury force-pushed the backport_ci_changes branch 3 times, most recently from 13c55e6 to 50a8e66 Compare March 6, 2024 15:41
@pamaury
Copy link
Contributor Author

pamaury commented Mar 6, 2024

I added a commit to remove the spi_device_ottf test, see commit for details.

Copy link
Contributor

@jwnrt jwnrt left a comment

Choose a reason for hiding this comment

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

I think removing the SPI device OTTF console test is fine since we don't have it in master anymore.

@pamaury pamaury force-pushed the backport_ci_changes branch from 50a8e66 to 86adb8f Compare March 7, 2024 11:11
jwnrt and others added 9 commits March 7, 2024 11:20
NOTE: this backport re-introduces the ROM and ROM e2e tests

Signed-off-by: James Wainwright <james.wainwright@lowrisc.org>
(cherry picked from commit 8845b9a)
At the moment, the sival job runs only `cw310_sival` jobs but not
`cw310_sival_rom_ext` which is not right. On the other hand,
the hyperdebug job runs all `hyper310` tests and that includes a
number of `cw310_sival` and `cw310_sival_rom_ext` tests so we run
some tests twice.

This commit changes removes the existing hyper310 job and split the
sival job into two subsets:
- sival_rom_ext tests,
- sival tests that do not have a corresponding sival_rom_ext tests.

Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
(cherry picked from commit ea8dab9)
Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
(cherry picked from commit 0b57c04)
This commit unifies cw310_params and cw310/hyper310_jtag_params by
integrating JTAG support to cw310_params as extra parameters.
The OpenOCD adapter config file (if any) is moved to execution
environments.

The intention is that one can choose to enable JTAG without having
to clear the bitstream. For backward compatibility of the rules,
*_jtag_params are set to clear the bitstream.

If test_harness is specified, then `--bitstream={bitstream}` is automatically
added to test_cmd. This was the behaviour for cw310_jtag_params, but is
generally useful for all other tests.

Co-authored-by: Amaury Pouly <amaury.pouly@lowrisc.org>
Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
Signed-off-by: Gary Guo <gary.guo@lowrisc.org>
(cherry picked from commit d809804)
This commit removes all usage of *_jtag_params and convert them to use
of `needs_jtag` and `changes_otp` (if necessary).

Signed-off-by: Gary Guo <gary.guo@lowrisc.org>
(cherry picked from commit ec74233)
Signed-off-by: James Wainwright <james.wainwright@lowrisc.org>
(cherry picked from commit 710c9f9)
Signed-off-by: James Wainwright <james.wainwright@lowrisc.org>
(cherry picked from commit 0fb33c8)
These flags need to come _before_ the subcommand, i.e.:

```
bazel --bazelrc=... cquery # VALID
bazel cquery --bazelrc=... # INVALID
```

which is done in this script by pushing flags that come before the
subcommand name into an array and re-applying them in the correct order
later.

If these Bash arrays are confusing and you've come across this commit in
a `git blame` then hopefully this will help:

1. Bash supports arrays defined using parentheses: `array=("foo" "bar")`
2. Arrays can't be passed as arguments to functions, they are expanded
   and get mixed into other arguments, so we use a global instead.
4. the syntax for expanding an array into its elements with correct
   quoting on each is: `"${array[@]}"`.

Signed-off-by: James Wainwright <james.wainwright@lowrisc.org>
(cherry picked from commit 4d88b95)
This tests causes issue in the CI due to using old rules and requiring
an odd combination of CW310 ROM + hyperdebug. It could be converted to
the new rules but since the SPI generic mode has been removed in master,
testing it in SiVal is pointless.

Signed-off-by: Amaury Pouly <amaury.pouly@lowrisc.org>
@pamaury pamaury force-pushed the backport_ci_changes branch from 86adb8f to 93f247d Compare March 7, 2024 11:20
@pamaury pamaury merged commit a099c0e into lowRISC:earlgrey_es_sival Mar 7, 2024
29 checks passed
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.

3 participants