´Use g++ and clang++ instead of gcc and clang as C++ compilers #4234

RCoeurjoly · 2024-02-24T08:36:16Z

This would solve the CXX and LD points of #2011.
LD is also changed to g++ when appropriate.

All pipelines pass in my fork (https://github.com/RCoeurjoly/yosys/actions).

whitequark

clang++ is not a linker, it is a compiler driver, so it should not be used in the $(LD) variable. Either put a linker (ld, lld, etc) in $(LD) or remove $(LD) altogether and change $(LDFLAGS) to $(LINKFLAGS) or something.

RCoeurjoly · 2024-02-24T11:25:33Z

It seems that clang++ or g++ can be used as the linker, specially if sanitizers are used: https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#usage.

I think it is not viable to use ld, because -rdynamic is supported in g++ and clang++ and not in ld, if I am not mistaken.
When I set LD = ld or remove $LD, I get the following error:
ld: unrecognised option: -rdynamic

…-> LINKFLAGS. CXX is clang++ or g++, not clang and gcc

whitequark · 2024-02-25T18:02:30Z

I think it is not viable to use ld

That is possible. In that case you should remove $(LD) entirely and use $(CXX) instead to link.

RCoeurjoly · 2024-02-25T21:27:45Z

I think it is not viable to use ld

That is possible. In that case you should remove $(LD) entirely and use $(CXX) instead to link.

Done. I had to change $LD for $CXX in passes/techmap/Makefile.inc, otherwise we would get the same linking error

ld: unrecognised option: -rdynamic

whitequark · 2024-02-25T21:31:10Z

Looks better, thank you. Now that we don't have $(LD), we should not use $(LDFLAGS) either; I propose $(LINKFLAGS). Similarly, instead of $(LDLIBS) we can use $(LIBS).

It looks like we have a bunch of $(filter-out -rdynamic) clauses that can probably be simplified by not adding -rdynamic by default in first place, but I'll take another look at that later, and it doesn't strictly speaking have to be a part of this PR.

RCoeurjoly · 2024-02-25T22:29:30Z

Looks better, thank you. Now that we don't have $(LD), we should not use $(LDFLAGS) either; I propose $(LINKFLAGS). Similarly, instead of $(LDLIBS) we can use $(LIBS).

It looks like we have a bunch of $(filter-out -rdynamic) clauses that can probably be simplified by not adding -rdynamic by default in first place, but I'll take another look at that later, and it doesn't strictly speaking have to be a part of this PR.

Done. I had to fix it in other Makefiles too.

Now two tests fail

one in macOS (https://github.com/RCoeurjoly/yosys/actions/runs/8041007987/job/21959774675#step:10:1562) and
one in Linux (https://github.com/RCoeurjoly/yosys/actions/runs/8041007983/job/21959774813#step:12:1444).

I have reproduced the error locally by running:
cd tests/various
bash run-test.sh

I get the following error:

make: *** [run-test.mk:456: async.sh] Error 127

I am a bit lost as to how to debug that failure.

povik · 2024-02-25T23:12:04Z

Now two tests fail

one in macOS (https://github.com/RCoeurjoly/yosys/actions/runs/8041007987/job/21959774675#step:10:1562) and

one in Linux (https://github.com/RCoeurjoly/yosys/actions/runs/8041007983/job/21959774813#step:12:1444).

Those happen to be the only CI jobs which run the testsuite (through make test), the other CI jobs build the binaries but don't run make test.

Looks like the fault is in tests/various/plugin.sh because yosys-config needs updating, see misc/yosys-config.in.

whitequark · 2024-02-26T08:06:53Z

misc/yosys-config.in

+		echo "    --ldflags     @LINKFLAGS@"
+		echo "    --ldlibs      @LIBS@"


I think we should rename them here as well, while providing compatibility:

Suggested change

echo " --ldflags @LINKFLAGS@"

echo " --ldlibs @LIBS@"

echo " --linkflags @LINKFLAGS@"

echo " --ldflags (alias of --linkflags)

echo " --linklibs @LIBS@"

echo " --ldlibs (alias of --linklibs)

whitequark · 2024-02-26T08:22:22Z

libs/ezsat/Makefile

 CXXFLAGS = -MD -Wall -Wextra -ggdb
 CXXFLAGS += -std=c++11 -O0
-LDLIBS = ../minisat/Options.cc ../minisat/SimpSolver.cc ../minisat/Solver.cc ../minisat/System.cc -lm -lstdc++
+LIBS = ../minisat/Options.cc ../minisat/SimpSolver.cc ../minisat/Solver.cc ../minisat/System.cc -lm -lstdc++


This is a completely unhinged definition of LIBS.

(I am not asking you to fix it; I am just stating my bewilderment.)

I was surprised too.

whitequark

Looks good to me, thank you a lot! The Makefiles are not perfect but they are at least not blatantly misleading anymore, so we are getting somewhere!!

RCoeurjoly · 2024-02-26T08:53:16Z

Looks good to me, thank you a lot! The Makefiles are not perfect but they are at least not blatantly misleading anymore, so we are getting somewhere!!

Thank you for your patience and handholding!

´Use g++ and clang++ instead of gcc and clang as C++ compilers

fe34aba

whitequark requested changes Feb 24, 2024

View reviewed changes

RCoeurjoly mentioned this pull request Feb 25, 2024

Begin supporting nix flakes build system #4238

Merged

RCoeurjoly added 6 commits February 25, 2024 16:43

LD is removed, we use CXX instead

a69a89f

We use CXX instead of LD for linking yosys-filterlib

033fa10

LDLIBS -> LIBS. LDFLAGS -> LINKFLAGS

dea4aea

Changes in libs, passes and tests Makefiles. LDLIBS -> LIBS. LDFLAGS …

4a2fb18

…-> LINKFLAGS. CXX is clang++ or g++, not clang and gcc

Changes in misc/yosys-config.in. LDLIBS -> LIBS. LDFLAGS -> LINKFLAGS.

cf7b6c6

Fix help of yosys-config.in and provide backward compatibility

5fa609b

whitequark reviewed Feb 26, 2024

View reviewed changes

whitequark approved these changes Feb 26, 2024

View reviewed changes

whitequark merged commit 639a8d8 into YosysHQ:master Feb 26, 2024
16 checks passed

whitequark mentioned this pull request Feb 26, 2024

compile fails on abc/src/misc/extra/extraUtilUtil.c: "ISO C++17 does not allow 'register' storage class specifier" #3881

Closed

AlexandreSinger mentioned this pull request Aug 19, 2024

./kernel/yosys.h:42:10: fatal error: 'map' file not found verilog-to-routing/vtr-verilog-to-routing#2697

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

´Use g++ and clang++ instead of gcc and clang as C++ compilers #4234

´Use g++ and clang++ instead of gcc and clang as C++ compilers #4234

RCoeurjoly commented Feb 24, 2024

whitequark left a comment •

edited

Loading

RCoeurjoly commented Feb 24, 2024

whitequark commented Feb 25, 2024

RCoeurjoly commented Feb 25, 2024

whitequark commented Feb 25, 2024 •

edited

Loading

RCoeurjoly commented Feb 25, 2024

povik commented Feb 25, 2024

whitequark Feb 26, 2024

whitequark Feb 26, 2024 •

edited

Loading

RCoeurjoly Feb 26, 2024

whitequark left a comment

RCoeurjoly commented Feb 26, 2024

-		echo "    --ldflags     @LINKFLAGS@"
-		echo "    --ldlibs      @LIBS@"
+		echo "    --linkflags   @LINKFLAGS@"
+		echo "    --ldflags     (alias of --linkflags)
+		echo "    --linklibs    @LIBS@"
+		echo "    --ldlibs      (alias of --linklibs)

´Use g++ and clang++ instead of gcc and clang as C++ compilers #4234

´Use g++ and clang++ instead of gcc and clang as C++ compilers #4234

Conversation

RCoeurjoly commented Feb 24, 2024

whitequark left a comment • edited Loading

Choose a reason for hiding this comment

RCoeurjoly commented Feb 24, 2024

whitequark commented Feb 25, 2024

RCoeurjoly commented Feb 25, 2024

whitequark commented Feb 25, 2024 • edited Loading

RCoeurjoly commented Feb 25, 2024

povik commented Feb 25, 2024

whitequark Feb 26, 2024

Choose a reason for hiding this comment

whitequark Feb 26, 2024 • edited Loading

Choose a reason for hiding this comment

RCoeurjoly Feb 26, 2024

Choose a reason for hiding this comment

whitequark left a comment

Choose a reason for hiding this comment

RCoeurjoly commented Feb 26, 2024

whitequark left a comment •

edited

Loading

whitequark commented Feb 25, 2024 •

edited

Loading

whitequark Feb 26, 2024 •

edited

Loading