ci: Test on development snapshots of GCC and Clang

real-or-random commented at 5:22 pm on May 13, 2023: contributor

This adds development snapshots of GCC and Clang to our Dockerfile, and – as a prototype – checks if CI likes this. I will later add a commit that actually defines some separate jobs.

It’s not clear yet if it’s a good idea to have these in a Dockerfile, or if this should be a separate Dockerfile, or no Docker CI environment at all. The advantage of the Docker CI environment is that the built Docker images are cached. But that’s also the disadvantage. We probably want to rebuild the Docker image from time to time, and we can’t do this automatically. The images are cached based on the contents of the Dockerfile. I think it’s possible to nuke the cache from the web interface, but we’d need to do this on a regular basis then.

Moreover, it’s not clear to me yet if these should be run as part of PRs and on master (as currently), or as cron/nightly builds or something else. I lean towards adding a few jobs to the current strategy (PRs+master) because I think moving some builds to nightly is an orthogonal question.

real-or-random force-pushed on May 13, 2023

sipa cross-referenced this on May 15, 2023 from issue Do not invoke fe_is_zero on failed set_b32_limit by sipa

sipa commented at 2:13 pm on May 15, 2023: contributor

See #1316 for a fix for one of the issues detected in the CI run here.

real-or-random commented at 9:00 pm on May 15, 2023: contributor

@sipa Do you have any idea about the other failure? https://cirrus-ci.com/task/4554836482457600?logs=test#L252 I have no idea what GCC is trying to tell us here… I don’t know which ret variable it is referring to? Maybe that’s a real false positive, or at least a bug in the diagnostic message? I mean, it’s an unreleased compiler in the end…

sipa commented at 9:06 pm on May 15, 2023: contributor

@real-or-random I can reproduce it locally, and can silence the warning by memset’ing the precj array in the function, but don’t understand the comment otherwise. Note that it’s in a mode where the precompute binary is being compiled with -DVERIFY, so it may include magnitude/normalize fields.

real-or-random force-pushed on May 16, 2023

real-or-random cross-referenced this on May 16, 2023 from issue Add a button to invalidate the cached Docker image by cjdb

real-or-random commented at 8:22 pm on May 16, 2023: contributor

I think it’s possible to nuke the cache from the web interface, but we’d need to do this on a regular basis then.

According to my experiments, clearing the cache is possible. (Only using a workaround, see https://github.com/cirruslabs/cirrus-ci-docs/issues/544, but I doubt that’s a problem.) I guess that’s good enough for starters. We could do it every few weeks, or easier, in every IRC meeting. What do you think?

real-or-random commented at 8:24 pm on May 16, 2023: contributor

@real-or-random I can reproduce it locally, and can silence the warning by memset’ing the precj array in the function, but don’t understand the comment otherwise. Note that it’s in a mode where the precompute binary is being compiled with -DVERIFY, so it may include magnitude/normalize fields.

After some deeper digging, I’ve managed to find a commit that improves our code and makes GCC happy here at the same time.

real-or-random cross-referenced this on May 17, 2023 from issue build: Enable -DVERIFY for precomputation binaries by real-or-random

real-or-random added the label assurance on May 17, 2023

real-or-random added the label ci on May 17, 2023

real-or-random added the label side-channel on May 17, 2023

real-or-random referenced this in commit 5f7903c73c on May 19, 2023

real-or-random referenced this in commit d373a7215b on May 23, 2023

hebasto cross-referenced this on Jun 25, 2023 from issue ci: Adjust Docker image to Debian 12 "bookworm" by hebasto

real-or-random referenced this in commit 799f4eec27 on Jun 27, 2023

hebasto commented at 7:56 am on June 27, 2023: member

Time to rebase? :)

real-or-random force-pushed on Jun 28, 2023

real-or-random force-pushed on Jun 29, 2023

real-or-random commented at 9:03 am on June 29, 2023: contributor

Ready for review.

Moreover, it’s not clear to me yet if these should be run as part of PRs and on master (as currently), or as cron/nightly builds or something else. I lean towards adding a few jobs to the current strategy (PRs+master) because I think moving some builds to nightly is an orthogonal question.

What I did now is something else:

Run all (14) Linux x86_64 jobs not only on gcc and clang, but additionally with gcc-snapshot and clang-snapshot (this gives us 28 additional jobs)
Instead of running the same 14 jobs on macOS jobs for both gcc and clang, reduce the macOS jobs to a more “curated set” of 8 jobs (this removes 28-8 = 20 jobs)

In total, the number of jobs is increased only by 8, and it is 89 after this PR. My rationale here is that I think it makes sense to test many jobs on snapshots, and I didn’t want to increase the number of jobs > 100.

Open to other suggestions and bikeshedding here. However, we should reconsider the macOS jobs anyway now that we can get Linux on native ARM64 too (https://github.com/bitcoin-core/secp256k1/pull/1163). The Linux tests are better (we get Dockerfile support, valgrind works, etc…) It still makes sense to test building on macOS, and it makes also sense to run a few tests there given that Apple’s clang is a bit modified. But given that our actual code has almost zero interaction with the OS, I’d prefer to run just a handful jobs on macOS, and see if we can get more ARM64 testing on Linux instead. For example, we could move some of the x86_64 jobs to ARM64.

real-or-random marked this as ready for review on Jun 29, 2023

hebasto commented at 4:41 pm on June 29, 2023: member

What the process is suggested to update compiler snapshots in the Docker image?

UPD. Asking because the PR description seems a bit vague about that.

real-or-random commented at 5:26 pm on June 29, 2023: contributor

What the process is suggested to update compiler snapshots in the Docker image?

Oh right, I should have pointed this out. As I said above, the image is cached based on a hash of the Dockerfile. So unless we change the Dockerfile,

As I wrote here, it’s possible for a maintainer to trigger a manual rebuild by clicking “re-run” on a Docker task. My suggestion is simply to do this manually, say in every (or every second) IRC meeting.

A similar alternative is that we have some kind of version number in the Dockerfile that we increment manually. We could again do this every two weeks or every four weeks. This is a bit inelegant because it needs a (trivial) PR every time we update. But the advantage of this is that we can postpone merging that PR if the update breaks…

I’m open to suggestions to automate this, but I currently don’t see a nice way to force this using Cirrus CI features.

hebasto commented at 7:59 pm on June 29, 2023: member

We could again do this every two weeks or every four weeks.

Maybe do this when new packages are available? For instance, the last time gcc snapshot was updated was on 2023-04-19.

real-or-random commented at 8:09 am on June 30, 2023: contributor

Maybe do this when new packages are available? For instance, the last time gcc snapshot was updated was on 2023-04-19.

Right, for GCC. The Clang packages are usually just a few days old: https://apt.llvm.org/, so I think something between “three months” and “a few days” is appropriate.

in ci/linux-debian.Dockerfile:37 in 3918c90a3a outdated

32+    ln -s /opt/gcc-latest/bin/gcc /usr/bin/gcc-snapshot
33+
34+# Install clang snapshot
35+RUN wget -qO- https://apt.llvm.org/llvm-snapshot.gpg.key | tee /etc/apt/trusted.gpg.d/apt.llvm.org.asc && \
36+    # Install add-apt-repository
37+    apt-get update && apt-get install --no-install-recommends -y software-properties-common && \

jonasnick commented at 9:02 am on July 3, 2023:

We don’t seem to use add-apt-repository.

real-or-random commented at 1:14 pm on July 3, 2023:

Indeed, this was an old line. Removed.

in ci/linux-debian.Dockerfile:32 in 3918c90a3a outdated

24@@ -23,6 +25,29 @@ RUN apt-get update && apt-get install --no-install-recommends -y \
25         sagemath
26 
27 WORKDIR /root
28+
29+# Install gcc snapshot
30+RUN wget --progress=dot:giga --content-disposition https://kayari.org/gcc-latest/gcc-latest.deb && \
31+    dpkg -i gcc-latest_*.deb && \
32+    ln -s /opt/gcc-latest/bin/gcc /usr/bin/gcc-snapshot

jonasnick commented at 9:07 am on July 3, 2023:

This installs gcc version 13.0.1 20230416. There are newer gcc snapshot sources available from the official mirrors, e.g., gcc-14-20230625. I suppose you didn’t pick this because we don’t want to get into the business of building the compiler (for now)?

real-or-random commented at 1:14 pm on July 3, 2023:

Hm, yeah, it seems that this build is already ~2.5 months old.

I suppose you didn’t pick this because we don’t want to get into the business of building the compiler (for now)?

Indeed.

The gcc team publishes development snapshots in source form (what you linked to). https://jwakely.github.io/pkg-gcc-latest/ picks these up and provides binary packages (what we install here). I wonder if there’s a regular cadence. Asked here: https://github.com/jwakely/pkg-gcc-latest/issues/14

jwakely commented at 1:34 pm on July 3, 2023:

It’s supposed to be every week.

real-or-random commented at 5:54 pm on July 3, 2023:

GCC 14 snapshot binaries are available now.

in .cirrus.yml:143 in 3918c90a3a outdated

144+    - env: {}
145+    - env: {WIDEMUL:  int64,  RECOVERY: yes, ECDH: yes, SCHNORRSIG: yes, ELLSWIFT: yes}
146+    - env: {WIDEMUL:  int64,  RECOVERY: yes, ECDH: yes, SCHNORRSIG: yes, ELLSWIFT: yes, CC: gcc}
147+    - env: {WIDEMUL: int128_struct, ECMULTGENPRECISION: 2, ECMULTWINDOW: 4}
148+    - env: {WIDEMUL: int128,                 ECDH: yes, SCHNORRSIG: yes, ELLSWIFT: yes}
149+    - env: {WIDEMUL: int128,  RECOVERY: yes, ECDH: yes, SCHNORRSIG: yes, ELLSWIFT: yes}

jonasnick commented at 9:11 am on July 3, 2023:

The first configuration seems to be a strict subset of the second. Maybe would be useful to have a CPPFLAGS: -DVERIFY build.

jonasnick commented at 9:17 am on July 3, 2023: contributor

What I did now is something else:

sgtm

In total, the number of jobs is increased only by 8, and it is 89 after this PR.

As far as I can see, it’s 91 now.

real-or-random force-pushed on Jul 3, 2023

real-or-random commented at 5:55 pm on July 3, 2023: contributor

Forced-push a rebased version to re-trigger a clean CI build. I think CI was a bit confused here when I clicked “re-run all tasks”.

jonasnick approved

jonasnick commented at 7:46 pm on July 3, 2023: contributor

ACK 6d75387888099816d97f841affc2a5a2dfd2abe9

in ci/linux-debian.Dockerfile:48 in 6d75387888 outdated

32+    ln -s /opt/gcc-latest/bin/gcc /usr/bin/gcc-snapshot
33+
34+# Install clang snapshot
35+RUN wget -qO- https://apt.llvm.org/llvm-snapshot.gpg.key | tee /etc/apt/trusted.gpg.d/apt.llvm.org.asc && \
36+    # Add repository for this Debian release
37+    . /etc/os-release && echo "deb http://apt.llvm.org/${VERSION_CODENAME} llvm-toolchain-${VERSION_CODENAME} main" >> /etc/apt/sources.list && \

MarcoFalke commented at 9:32 am on July 4, 2023:

Where is VERSION_CODENAME set?

jonasnick commented at 9:40 am on July 4, 2023:

It’s set by debian’s os-release (https://manpages.debian.org/unstable/systemd/os-release.5.en.html).

hebasto commented at 1:35 pm on July 4, 2023: member

re: #1313 (review)

I suppose you didn’t pick this because we don’t want to get into the business of building the compiler (for now)?

Suggesting to do not rely on the https://github.com/jwakely/pkg-gcc-latest project at all.

Here is a proof of concept of building gcc from the source:

 0--- a/ci/linux-debian.Dockerfile
 1+++ b/ci/linux-debian.Dockerfile
 2@@ -26,9 +26,15 @@ RUN apt-get update && apt-get install --no-install-recommends -y \
 3 
 4 WORKDIR /root
 5 
 6-# Install gcc snapshot
 7-RUN wget --progress=dot:giga --content-disposition https://kayari.org/gcc-latest/gcc-latest.deb && \
 8-    dpkg -i gcc-latest_*.deb && \
 9+# Build and install gcc snapshot
10+ARG GCC_SNAPSHOT_VERSION=gcc-14-20230702
11+RUN wget --progress=dot:giga --content-disposition https://gcc.gnu.org/pub/gcc/snapshots/LATEST-14/${GCC_SNAPSHOT_VERSION}.tar.xz && \
12+    tar xf ${GCC_SNAPSHOT_VERSION}.tar.xz && \
13+    apt-get update && apt-get install --no-install-recommends -y libgmp-dev libmpfr-dev libmpc-dev flex && \
14+    mkdir gcc-build && cd gcc-build && \
15+    ../${GCC_SNAPSHOT_VERSION}/configure --prefix=/opt/gcc-latest --enable-languages=c --disable-bootstrap --disable-multilib --without-isl && \
16+    make -j $(nproc) && \
17+    make install && \
18     ln -s /opt/gcc-latest/bin/gcc /usr/bin/gcc-snapshot
19 
20 # Install clang snapshot

One of the benefits is building for C language only.

real-or-random commented at 9:50 pm on July 5, 2023: contributor

I suppose you didn’t pick this because we don’t want to get into the business of building the compiler (for now)?

Suggesting to do not rely on the jwakely/pkg-gcc-latest project at all.

Hm, yeah. I assumed it’s more hassle. If it’s just these 6 lines and runs in ~30 min more, then we should indeed consider this.

--without-isl

AFAIU this makes some more optimizations possible, which I think are only enabled at -O3. But I’m not sure and who knows if some of these will later be enabled in -O2. Is it difficult to build with ISL support?

jwakely commented at 10:51 pm on July 5, 2023: none

AFAIU this makes some more optimizations possible, which I think are only enabled at -O3.

No, they’re not enabled by -O3, they’re only enabled if you explicitly request them (and if GCC was built with ISL).

But I’m not sure and who knows if some of these will later be enabled in -O2.

They won’t be.

Is it difficult to build with ISL support?

No, but the additional optimization passes it enables are not used by default and do not improve performance of most code anyway.

hebasto commented at 1:41 pm on July 6, 2023: member

Agree with #1313 (comment).

To be precise, ISL enables optimization options as follows:

-ftree-loop-linear
-floop-strip-mine
-floop-block
-fgraphite-identity

None of them are managed by the -O<N> options.

real-or-random force-pushed on Jul 6, 2023

real-or-random commented at 3:00 pm on July 6, 2023: contributor

Force-pushed and included @hebasto’s suggestion.

in ci/linux-debian.Dockerfile:30 in 00d5d3b4d4 outdated

24@@ -23,6 +25,33 @@ RUN apt-get update && apt-get install --no-install-recommends -y \
25         sagemath
26 
27 WORKDIR /root
28+
29+# Build and install gcc snapshot
30+ARG GCC_SNAPSHOT_VERSION=gcc-14-20230702

jwakely commented at 3:02 pm on July 6, 2023:

This will break at the weekend, when the LATEST-14 symlink no longer points to the 20230702 snapshot. It will point to a different date, and so the name of the tarball will be different.

jwakely commented at 3:05 pm on July 6, 2023:

You could replace the LATEST-14 directory with 14-20230702 which will not be invalidated every week, but then your CI will just be using the same snapshot forever.

thesamesam commented at 3:12 pm on July 6, 2023:

Or just use the ones built for this purpose 😄

jwakely commented at 3:52 pm on July 6, 2023:

It also seems quite wasteful to keep building the same snapshots again and again, but that is on brand for bitcoin.

real-or-random commented at 3:53 pm on July 6, 2023:

This will break at the weekend, when the LATEST-14 symlink no longer points to the 20230702 snapshot. It will point to a different date, and so the name of the tarball will be different.

Oh sure…

You could replace the LATEST-14 directory with 14-20230702 which will not be invalidated every week, but then your CI will just be using the same snapshot forever.

Indeed. Hm. The ability to pin a specific snapshot is probably useful. This means we’d need to update the dockerfile regularly (e.g., once per month) with a PR. I think that’s okay. It’s just a bit inconsistent if we don’t do the same for the clang snapshots…

For now, I simply replaced this with a version that downloads the latest snapshot. (Using wget’s recursion feature that recurses into downloading links.) I think any approach will be good enough as an initial attempt. We’ll need to see how often we run into problems.

Or just use the ones built for this purpose smile

You mean those at https://jwakely.github.io/pkg-gcc-latest/ ?

real-or-random commented at 6:03 pm on July 6, 2023:

It also seems quite wasteful to keep building the same snapshots again and again,

Note that our CI infrastructure builds this once as a docker image, and then every CI jobs runs in a new container using this prebuilt image, which has the GCC snapshot preinstalled then. The docker image is only rebuilt if we update the Dockerfile, or otherwise trigger a rebuild manually. That’s a pretty neat feature of Cirrus CI, see https://cirrus-ci.org/guide/docker-builder-vm/#dockerfile-as-a-ci-environment

real-or-random force-pushed on Jul 6, 2023

hebasto approved

hebasto commented at 4:27 pm on July 6, 2023: member

ACK 53980dfdb5f62ae3491a8c38c94775839925b913.

https://api.cirrus-ci.com/v1/task/6462209514012672/logs/test.log:

 0...
 1+ gcc-snapshot -v
 2Using built-in specs.
 3COLLECT_GCC=gcc-snapshot
 4COLLECT_LTO_WRAPPER=/opt/gcc-snapshot/libexec/gcc/x86_64-pc-linux-gnu/14.0.0/lto-wrapper
 5Target: x86_64-pc-linux-gnu
 6Configured with: ../gcc-14-20230702/configure --prefix=/opt/gcc-snapshot --enable-languages=c --disable-bootstrap --disable-multilib --without-isl
 7Thread model: posix
 8Supported LTO compression algorithms: zlib zstd
 9gcc version 14.0.0 20230702 (experimental) (GCC) 
10...

https://api.cirrus-ci.com/v1/task/5054834630459392/logs/test.log:

 0...
 1+ clang-snapshot -v
 2Debian clang version 17.0.0 (++20230624112259+eaaacc3c651e-1~exp1~20230624112316.43)
 3Target: x86_64-pc-linux-gnu
 4Thread model: posix
 5InstalledDir: /usr/bin
 6Found candidate GCC installation: /usr/bin/../lib/gcc-cross/i686-linux-gnu/12
 7Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/12
 8Selected GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/12
 9Candidate multilib: .;@m64
10Selected multilib: .;@m64
11...

ci: Install development snapshots of gcc and clang

TODO: Make sure the Docker image is actually rebuild

1deecaaf3b

ci: Add x86_64 Linux tasks for gcc and clang snapshots 609093b387

ci: Reduce number of macOS tasks from 28 to 8 e9e9648219

ci: Fix typo in comment 981e5be38c

real-or-random force-pushed on Jul 6, 2023

real-or-random commented at 6:20 pm on July 6, 2023: contributor

Added checking of SHA512 checksum for the GCC download. @hebasto sorry to invalidate your ACK,

hebasto approved

hebasto commented at 6:30 pm on July 6, 2023: member

re-ACK 981e5be38c492f0c0230fbe61be555d157380331

Added checking of SHA512 checksum for the GCC download.

Nice.

jonasnick approved

jonasnick commented at 2:41 pm on July 13, 2023: contributor

ACK 981e5be38c492f0c0230fbe61be555d157380331

jonasnick merged this on Jul 13, 2023

jonasnick closed this on Jul 13, 2023

hebasto cross-referenced this on Jul 13, 2023 from issue Fix symbol visibility issues, add test for it by hebasto

fanquake referenced this in commit 56c05c5ec4 on Jul 17, 2023

fanquake referenced this in commit ff061fde18 on Jul 18, 2023

hebasto referenced this in commit 270d2b37b8 on Jul 21, 2023

jonasnick cross-referenced this on Jul 27, 2023 from issue Upstream PRs 1357, 1345, 1358, 1368, 1369, 1323, 1364, 1370, 1367, 1344, 1371, 1363, 1340, 1366, 1313, 1378, 1298, 1382, 1383, 1384, 1386 by jonasnick

delta1 referenced this in commit 3f32c20932 on Aug 8, 2023

delta1 referenced this in commit 31ac0c1081 on Aug 31, 2023

real-or-random added the label needs-changelog on Aug 31, 2023

real-or-random removed the label needs-changelog on Sep 4, 2023

ci: Test on development snapshots of GCC and Clang #1313