yosys: cleanup of ghdl-yosys-plugin config/envvars

[umarcor]

It seems that yosys_ghdl.diff points to a different *.link file than the upstream: https://github.com/ghdl/ghdl-yosys-plugin/blob/master/yosys.diff. I thought that GHDL_LDLIBS might have been a workaround because of having overlooked that change.

[edbordin]

Hmm, this PR didn't seem to build but it's probably just because $(shell cat $(GHDL_LIB_DIR)/ghdl.link) in the makefile didn't ever seem to be working correctly. GHDL_LDLIBS was a workaround to replace the newlines in the .link file with spaces so that the entire contents of the file make it to the command line. On windows it also handles path conversion from unix paths to windows paths. Confusingly even though the shell uses unix paths, most of the mingw toolchain seems to use windows paths (I think normally mingw32-make handles this).

It probably would have been good if I had managed to use yosys.diff from upstream, but I think I did end up making a couple of patches to the patch... Unfortunately I won't be able to spend much time on this for a few weeks but I will at least try to review anything you come up with here.

[umarcor]

Hmm, this PR didn't seem to build but it's probably just because $(shell cat $(GHDL_LIB_DIR)/ghdl.link) in the makefile didn't ever seem to be working correctly. GHDL_LDLIBS was a workaround to replace the newlines in the .link file with spaces so that the entire contents of the file make it to the command line. On windows it also handles path conversion from unix paths to windows paths. Confusingly even though the shell uses unix paths, most of the mingw toolchain seems to use windows paths (I think normally mingw32-make handles this).

The point is that I built it successfully on MSYS2 (both MINGW32 and MINGW64) without having to work around reading the *.link file. See, for instance: https://github.com/umarcor/yosys/actions/runs/450104465. Moreover, the patch was upstreamed from ghdl-yosys-plugin to Yosys (see YosysHQ/yosys#2514). Therefore, we should be able to handle this properly.

It probably would have been good if I had managed to use yosys.diff from upstream, but I think I did end up making a couple of patches to the patch...

I now removed the diffs from upstream (which were merged into Yosys) and I left the additional patches, which I think are related to the static build. I also renamed it to yosys.diff, because I'm not sure about it being related to ghdl-yosys-plugin any more.

[edbordin]

Thanks for all your work upstream! I have to admit I haven't been the best open source citizen on that side of things... This looks close now but the GHDL synth smoke test is failing. Looks like YOSYS_ENABLE_GHDL needs to be defined and the Makefile somehow isn't doing it. This code path is running and giving the error message "This version of Yosys is built without GHDL support" https://github.com/ghdl/ghdl-yosys-plugin/blob/e43666c176544ba8bac0d1b89b6ac8e19abd2c28/src/ghdl.cc#L1225

[umarcor]

There were two problems:

• Calling $MAKE config-* overwrites Makefile.conf, and I was creating it before. I fixed it by moving the ghdl-yosys-plugin customisation to a function.
• See makefile: fix GHDL vars, replace GHDL_DIR with GHDL_PREFIX yosys#2515. Therefore, I was unsuccessfuly trying values for GHDL_DIR and GHDL_PREFIX. Both of them need to be used until that PR is merged. Hence, let's keep this on hold, even if green.

[umarcor]

It seems that CXXFLAGS is properly set (-DYOSYS_ENABLE_GHDL is found in the logs), but the LDLIBS additions are ignored?

[edbordin]

This is probably a bit higher priority now as your change upstream in yosys to use GHDL_PREFIX instead of GHDL_DIR appears to have broken the build on the main branch here.

It seems that CXXFLAGS is properly set (-DYOSYS_ENABLE_GHDL is found in the logs), but the LDLIBS additions are ignored?

I'm not sure the way I originally got this working was very methodical unfortunately...

[umarcor]

@edbordin I will fix this today. If I don't get it to work as clean as I'd like, I will revert to your solution but updating GHDL_DIR to GHDL_PREFIX.

[edbordin]

@umarcor yep sounds good! Thanks for volunteering your time so readily. I just opened #69 for the sake of tracking the issue. I agree that it might be easiest to just patch the existing solution so that your PR here doesn't have to be rushed, but do what works best for you

[umarcor]

I'm getting close. Linux and macOS builds are working. Windows is not there yet.