Switch compilation from C++17 to C++20. #4347
Conversation
|
I started looking at this some time ago in #4297. I was probably too sneaky though. Still not complete, there is quite lot of things that break and I don't have much time for it now. The main problem is clashing operators. |
I can close this PR in favor of yours. I just wanted to see what needs to be done and it seems moving up is non-trivial. |
OK. Yes, it does not seem as easy as was the move to C++17. |
|
There is not much missing for C++20 support. It looks like there is just one small failure left. |
|
Hmm, the error is in boost, that might be hard to fix unless it is our bad usage of boost of course :-). Seems to be related to a particular version as it fails just in the sanitizers build. What are your thoughts on switching? Do you think it is time for that? We don't get many goodies until we bump minimal required GCC, but even some things from GCC 9 are nice. It would also unlock C++20 for downstreams that have newer compilers. On the other hand, we would be having a more experimental setup, with GCC that is far from stable on C++20. |
The boost version should be the same for Ubuntu 20.04 but this compiler is Clang. Maybe that is the reason.
I am happy to switch, there are some folks that have been waiting for us to upgrade to C++20 already. We can try to be conservative with the features we introduce and keep Ubuntu 20.04 as the minimum supported version until it hits EOL. |
Cherrypicking #4663 fixes the issue. There were two more small fixes required which I can fork into a separate PR. |
Personally I'm OK with the operator fixes to be there, but the boost should be separate. |
|
I've just notice that |
Yes, the changes were for testing purposes, I will move them into separate PRs. |
63d7c31 to
07beb28
Compare
|
It looks like there is very little that can be done to fix the compilation issue sans fixing boost. I am not sure why this pops up with clang only though. |
Kinda scared of that one and C++20, ha. |
|
FYI, locally I am still hitting the boost bug from #4874.
yes, the bf-asm is huge and I don't know what could be hiding there :-D. And C++20 is not 100 % backward compatible, especially when considering the various bugs that creep in g++ versions we need to 8000 support. |
|
We will also need to run this on tofino, currently I am getting problems in |
Personally, I still think the best way to solve this is to build with our own boost version. |
Or at least require boost version w/o the bug? Do we know what is the minimum Ubuntu version that works here? |
iirc it was Boost 1.76+ https://stackoverflow.com/a/67702145, which is supplied with Ubuntu 22.04 and above. Since we are deprecating Ubuntu 20.04 we should not run into this anymore. |
ff023c7 to
c13b298
Compare
51bcc1b to
5ddbf9a
Compare
| @@ -160,7 +160,7 @@ const IR::Expression *CanonGatewayExpr::postorder(IR::Operation::Relation *e) { | |||
| Pattern::Match<IR::Expression> e1, e2; | |||
| Pattern::Match<IR::Constant> k1, k2; | |||
| int width = safe_width_bits(e->left->type); | |||
| if (((e1 & k1) == k2).match(e) || ((e1 & k1) != k2).match(e)) { | |||
| if (((e1 & k1) == k2).match(e) || ((e1 & k1) == k2).match(e)) { | |||
There was a problem hiding this comment.
This changes what is being matched (from an IR::Neq to an IR::Equ) to completely change the behavior of the code. It seems just wrong
| @@ -3111,7 +3111,7 @@ const IR::Node *RemoveUnnecessaryActionArgSlice::preorder(IR::Slice *sl) { | |||
| // Simplify "actionArg != 0 ? f0 : f1" to "actionArg ? f0 : f1" | |||
| const IR::Node *SimplifyConditionalActionArg::postorder(IR::Mux *mux) { | |||
| Pattern::Match<IR::MAU::ActionArg> aa; | |||
| if ((0 != aa).match(mux->e0)) mux->e0 = aa; | |||
| if ((aa.operator!=(0)).match(mux->e0)) mux->e0 = aa; | |||
There was a problem hiding this comment.
This should not be necessary.
| if (Device::statefulAluSpec().CmpMask && ((e1 & k) == e2).match(rel) && !k->fitsUint() && | ||
| !k->fitsInt()) { | ||
| if (Device::statefulAluSpec().CmpMask && ((e1 & k).operator==(e2)).match(rel) && | ||
| !k->fitsUint() && !k->fitsInt()) { |
There was a problem hiding this comment.
This should not be necessary
aa8e764 to
c50c865
Compare
f71b522 to
221a4c6
Compare
Signed-off-by: fruffy <fruffy@nyu.edu>
Signed-off-by: fruffy <fruffy@nyu.edu>
Signed-off-by: fruffy <fruffy@nyu.edu>
Signed-off-by: fruffy <fruffy@nyu.edu>
C++20 is now supported in most major distributions and compilers and there are several features that are useful for us to have. Let's switch to C++20.