supranational / pasta-msm

High-performance Multi-scalar Multiplication for Pasta curves
Apache License 2.0
21 stars 11 forks source link

Current `main` doesn't compile #6

Closed leonardoalt closed 1 year ago

leonardoalt commented 1 year ago

Hi, since a few days (maybe a week) I can't compile pasta-msm (and anything that depends on it). The error I get is while compiling semolina: https://gist.github.com/leonardoalt/79f2984dcd6cead9942deebba1334e22, likely related to https://github.com/supranational/semolina/issues/1. Semolina itself compiles fine in isolation, but fails when imported by pasta-msm.

leonardoalt commented 1 year ago

Some more info on this: the problem seems to be caused by a combination of sppark and semolina which happens here. The C++ error on the semolina code seems legit, it also happens when I manually run c++ https://gist.github.com/leonardoalt/c21a2c191db4d735ebdc5e1c49988310.

It's basically complaining that abort() doesn't exist. Adding #include <cstdlib> to https://github.com/supranational/semolina/blob/main/src/pasta_t.hpp makes it compile.

What I still don't understand is why this broke now, since it was working a few days ago and that code in particular doesn't seem to have been modified recently.

dot-asm commented 1 year ago

The question I struggle with is how come I didn't catch this problem. I've now added the cf4972c to exercise it on Windows and MacOS(*)... I've caught couple of other problems, but not this one... All this is not to say that the problem won't be fixed, rather how to prevent something like this from happening in the future. Could you elaborate on your setup? What's you Linux distribution? CUDA version?

(*) On Linux it builds even CUDA, but naturally can't actually exercise it. At least we know that it compiles.

leonardoalt commented 1 year ago

I didn't quite get whether you were able to replicate it? Here's the minimal sequence of commands that lead to the failure for me:

$ git clone https://github.com/supranational/pasta-msm.git pasta-msm-test
$ cd pasta-msm-test
$ cargo cache -a # this was needed to clear the cache from previous compilations which I did have
$ cargo check

Setup:

Arch Linux No CUDA Rust 1.69.0

I have a weird suspicion that GCC changed some stuff in the standard libraries from version 12 -> 13 or something like that. Yesterday I've seen a similar error ("uint32_t not found, did you forget to include \<cstdint>?") in another completely unrelated C++ repo.

dot-asm commented 1 year ago

I haven't ever noticed it, which obviously means that I've failed to reproduce it on my computer. And I've added Github Actions to the list of systems where the problem doesn't show up.

Arch Linux ... weird ... uint32_t

Thanks! I'll double-check everything :-)

leonardoalt commented 1 year ago

Thanks! A better hint: https://gcc.gnu.org/gcc-13/porting_to.html I'm fairly confident this is the cause. Arch Linux's Pacman uses GCC 13 as its latest (from a few weeks ago I think), whereas Ubuntu is on 12. I tried the same steps I sent you on Ubuntu and everything works fine.

dot-asm commented 1 year ago

cargo update to fix the problem. Thanks for the report!

leonardoalt commented 1 year ago

Amazing, thanks!!