XSim: better Windows support + add xcix IP import #1246

oletf · 2023-11-16T10:48:47Z

Follow-up to #1234

Context, Motivation & Description

Context : from XSim on Windows : getting it to work as expected through cmd or MSYS2(MINGW64) #1234 (comment)
Description :
- calls to Vivado generated scripts are called from cmd native shell
- msys2 still needed but only to build spinal_xsim.dll
- wdb file moved to xsim subfolder to avoid Vivado cluttering Project root folder when opening wdb viewer
- added xcix import along xci IP

Impact on code generation

None

Checklist

~~- [ ] Unit tests were added~~

XSim Simulation behavior changes started to be documented:
- on RTD : Describe the usage of XSim. SpinalDoc-RTD#230

Readon · 2023-11-16T15:10:55Z

Please notice the fact that many people is using SpinalHDL on Windows without MSYS2 environment. So if it is on Windows there might be chance that MSYS2_ROOT is not there at all.

oletf · 2023-11-16T16:13:38Z

The only files I changed are related to XSim usage,
so if they currently don't use XSim for simulation they won't see any change as those file's code won't run more as they currently do.

But I admit that for any Windows user who wants to use XSim, the spinal_xsim.dll needs to be compiled, so it'll be necessary for the user to have at least some g++ or whatever toolchain which would be able to build it.

I kept using MSYS2 shell in this PR because that's what was used previously already.

I found some mingw in %VIVADO_HOME%\tps\mingw which has some g++ but I haven't checked its limitations.

Readon · 2023-11-16T16:49:20Z

sim/src/main/scala/spinal/sim/XSimBackend.scala

+    if (isWindows) {
+      val msys2ShellPath =
+        sys.env.getOrElse("MSYS2_ROOT", "C:\\msys64") + "\\msys2_shell.cmd"
+      if (!new File(msys2ShellPath).exists) {


this part is what I concerned.

indeed I completely missed that this would fail on msys2 shell, thanks for spotting it!

could be changed to msys2Shell gets :

sh if sbt run by msys (check with sys.env.get("MSYSTEM") ?)

the full %MSYS2_ROOT%\\msys2_shell.cmd -defterm -here -no-start -mingw64 if ran by cmd

would that be good enough ?

pretty unsure about checking msys vs cmd with only MSYSTEM envvar,
thought about checking if sh exists, but there can be some other sh in cmd PATH

Readon · 2023-11-16T16:51:59Z

sim/src/main/scala/spinal/sim/XSimBackend.scala

      if (isWindows) {
-        s"sh -c \'${baseCommand}\'"
+        s"cmd /c $baseCommand"


so do you use cmd console by default?

yes, as mentioned in the issue a few times :
my goal was using native cmd console by default to call everything that can be :
vivado.bat, and its generated compile.bat and elaborate.bat :
so whatever if user starts sbt from cmd or from msys shell :

all .bat will be run by cmd

the g++ command to build spinal_xsim.dll is run by msysShell

Readon · 2023-11-16T16:53:41Z

sim/src/main/scala/spinal/sim/XSimBackend.scala

@@ -111,6 +124,15 @@ class XSimBackend(config: XSimBackendConfig) extends Backend {
      throw new Exception(message)
    }
  }
+  def doCmdMys2(command: String, cwd: File, message : String) = {
+    val cmd_msys2 = s""" $msys2Shell -defterm -here -no-start -mingw64 -c "$command" """.trim


this command would not use env variable in msys if you start it through cmd shell on Windows.

yes, because on windows the build of spinal_xsim.dll depends only on the VIVADO_HOME env var to copy locally needed include files.

This behavior was already there before those commits, and I kept it,
as it allows to have this only step which actually doesn't depends on any PATH (cmd or msys) env var.

Readon · 2023-11-16T16:56:30Z

there are normally 3 ways to use.

on linux
on Windows, use cmd shell
on Windows, use bash of msys2

andreasWallner · 2023-11-19T01:46:18Z

there are normally 3 ways to use.

1. on linux 
2. on Windows, use cmd shell 
3. on Windows, use bash of msys2

For the windows cases: aren't they the same in the sense that g++ must always run in the msys environment (since the compiler is installed there), whereas vivado "must" always run in the cmd environment (since there are no environment settings files for bash, only settings64.bat)?

I think from that point it makes sense to call g++ via msys and vivado via cmd.

Readon · 2023-11-19T01:55:45Z

there are normally 3 ways to use.
1. on linux 
2. on Windows, use cmd shell 
3. on Windows, use bash of msys2
For the windows cases: aren't they the same in the sense that g++ must always run in the msys environment (since the compiler is installed there), whereas vivado "must" always run in the cmd environment (since there are no environment settings files for bash, only settings64.bat)?

I think from that point it makes sense to call g++ via msys and vivado via cmd.

you are using method 2.

I am currently use the MSYS2 shell to start Vivado, the description in SpinalHDL/SpinalDoc-RTD#230 has been tested without cmd at all. and also with the all-in-one msys2 bundle.

A easy and unify method is prefered, because the environment of Windows norrmally a mess while the number of programs increasing.

- always use cmd to call Vivado related scripts - always using msys's g++ to build XSIIFace dll

andreasWallner · 2023-12-10T00:28:03Z

@Readon anything else from your side, from my POV we can merge this (I tried it on Linux)

Readon · 2023-12-11T05:05:35Z

@Readon anything else from your side, from my POV we can merge this (I tried it on Linux)

I still could not run this under msys2 environment.

Readon · 2023-12-11T05:21:58Z

@Readon anything else from your side, from my POV we can merge this (I tried it on Linux)

I still could not run this under msys2 environment.

What I am using is the portable environment installed by MSYS2-installer. I think in MSYS2 there sould be no assumption that there would be a cmd.exe exist.

Readon · 2023-12-11T05:36:47Z

I also tested in the cmd console it still prompt that "Please Setup you MSYS2_ROOT Environment Variable." after I have set it by "set MSYS2_ROOT="xxxxxxx"". I have to define all environment variable in Windows System's advance settings. It might be not good if I just want to run it once, but pollute my whole environment variables.

After that, I still get " Generation of vivado script failed" error with backtrace like below.

[error]         at spinal.sim.XSimBackend.doCmd(XSimBackend.scala:125)
[error]         at spinal.sim.XSimBackend.doCmdVivado(XSimBackend.scala:141)
[error]         at spinal.sim.XSimBackend.generateScript(XSimBackend.scala:245)

Readon · 2023-12-11T08:51:43Z

One more thing, if the xcix is supported after this PR, how about add a minimal test on it? So that we could test this each time the CI runs.

oletf · 2023-12-11T09:46:10Z

What I am using is the portable environment installed by MSYS2-installer. I think in MSYS2 there sould be no assumption that there would be a cmd.exe exist.

For any MSYS2 installation, I assumed that we're on Windows, and that the cmd.exe native windows shell existing since the very beginnings of the OS is always present.
Also the fact that launching a .bat is not possible without a minimal cmd /c call, and that even calling a .bat from msys2 will call it through a cmd subprocess.
Or did you refer to the msys2_shell.cmd script which maybe could not have been yet added in your current msys2 version ?

"set MSYS2_ROOT="xxxxxxx""

Windows cmd syntax is very misleading on many things,
if you got spaces in the path you want to set I think it's better to do it by set "MSYS2_ROOT=xxxxxxx" (see this for details).
I always forget this it's terrible...

I have to define all environment variable in Windows System's advance settings.

Which other env var than the MSYS2_ROOT and VIVADO_HOME did you have to set to reach to the errors you got ?
Now I'm actually unsure whether java's sys.env.get gets Windows or MSYS2 env var when the java process in running as child of msys2 shell.

Writing this paragraph above I now realize that the failing at XSimBackend.scala:245 might actually be either :

caused by either msys2 env being different from mine
Vivado path resolution failing at some point
both

So yeah I think I cannot properly test running this by sbt from msys2, since my sbt install is in the windows side,
and msys2 is just used for g++ in my case.
I might have overlooked another thing for it to work in your specific case,
please let me know how I can try to reproduce your case on my side !

oletf · 2023-12-11T09:50:24Z

One more thing, if the xcix is supported after this PR, how about add a minimal test on it? So that we could test this each time the CI runs.

I admit I just added xcix because I needed it in my project, and just adding it to the IP import list didn't cause any problem from Vivado's pov.

Not sure about how to add a proper test for this (still lacking experience in scala testing workflow),
is there an xci test which already exists I could refer to ?

Readon · 2023-12-11T10:06:37Z

For any MSYS2 installation, I assumed that we're on Windows, and that the cmd.exe native windows shell existing since the very beginnings of the OS is always present. Also the fact that launching a .bat is not possible without a minimal cmd /c call, and that even calling a .bat from msys2 will call it through a cmd subprocess. Or did you refer to the msys2_shell.cmd script which maybe could not have been yet added in your current msys2 version ?

I suppose it is not a good assumption. I run the TestXSim in a standalone MSYS2 environment, there is no cmd. However, I pushed a PR on your repos, may be that helps. And it can call the compile.bat and elaborate.bat files, while you have used setEnvAndRunCmd.sh first by msys2_shell.cmd script.

Windows cmd syntax is very misleading on many things, if you got spaces in the path you want to set I think it's better to do it by set "MSYS2_ROOT=xxxxxxx" (see this for details). I always forget this it's terrible...

Yes, this might be the problem.

Which other env var than the MSYS2_ROOT and VIVADO_HOME did you have to set to reach to the errors you got ? Now I'm actually unsure whether java's sys.env.get gets Windows or MSYS2 env var when the java process in running as child of msys2 shell.

It's weird that it can get VIVADO_HOME when I define it in console but in the system. And fails to get for MSYS2_ROOT.

So yeah I think I cannot properly test running this by sbt from msys2, since my sbt install is in the windows side, and msys2 is just used for g++ in my case. I might have overlooked another thing for it to work in your specific case, please let me know how I can try to reproduce your case on my side !

If you want you can install the MSYS2-Installer, and define the environment variable in that. It can work either on current upstream code, and on the PR I have pushed on your repo.

The only problem remains is that I could not get the Windows starter, by cmd, to work now.

Readon · 2023-12-11T10:07:38Z

One more thing, if the xcix is supported after this PR, how about add a minimal test on it? So that we could test this each time the CI runs.

I admit I just added xcix because I needed it in my project, and just adding it to the IP import list didn't cause any problem from Vivado's pov.

Not sure about how to add a proper test for this (still lacking experience in scala testing workflow), is there an xci test which already exists I could refer to ?

I think there is no, and the xcix is normally a commercially used one, I am not sure if you can upload it here.

make the standalone MSYS2 runs.

Readon · 2023-12-31T09:18:19Z

Sorry for the late response.
I have tested the latest version on my machine. It works on MSYS2 standalone environment alright.

However, on Windows cmd & MSYS2 mixed environment it still report "UnsatisfiedLinkError", even when I defined VIVADO_HOME and add %VIVADO_HOME%\lib\win64.o to my PATH.
Almost the same as #1234 states.

Readon · 2024-01-22T03:09:14Z

Sorry for the late response. Thanks for the contribution.

oletf added 3 commits November 16, 2023 11:27

XSim: windows support through msys2

88ce8ac

XSim: import xcix IP

55952cc

XSim: replace asserts by exceptions on vivadoHome and msys2Shell

f837885

Readon reviewed Nov 16, 2023

View reviewed changes

oletf mentioned this pull request Nov 27, 2023

XSim on Windows : getting it to work as expected through cmd or MSYS2(MINGW64) #1234

Closed

XSim: Windows : fix detection sbt shell detection

10280ed

- always use cmd to call Vivado related scripts - always using msys's g++ to build XSIIFace dll

make the standalone MSYS2 runs.

bba6e91

Merge pull request #1 from Readon/msys2

6659822

make the standalone MSYS2 runs.

Readon merged commit 26c5473 into SpinalHDL:dev Jan 22, 2024
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

XSim: better Windows support + add xcix IP import #1246

XSim: better Windows support + add xcix IP import #1246

oletf commented Nov 16, 2023

Readon commented Nov 16, 2023

oletf commented Nov 16, 2023

Readon Nov 16, 2023

oletf Nov 16, 2023 •

edited

Readon Nov 16, 2023

oletf Nov 16, 2023 •

edited

Readon Nov 16, 2023

oletf Nov 16, 2023

Readon commented Nov 16, 2023

andreasWallner commented Nov 19, 2023

Readon commented Nov 19, 2023

andreasWallner commented Dec 10, 2023 •

edited

Readon commented Dec 11, 2023

Readon commented Dec 11, 2023

Readon commented Dec 11, 2023

Readon commented Dec 11, 2023

oletf commented Dec 11, 2023

oletf commented Dec 11, 2023

Readon commented Dec 11, 2023

Readon commented Dec 11, 2023

Readon commented Dec 31, 2023

Readon commented Jan 22, 2024

XSim: better Windows support + add xcix IP import #1246

XSim: better Windows support + add xcix IP import #1246

Conversation

oletf commented Nov 16, 2023

Context, Motivation & Description

Impact on code generation

Checklist

Readon commented Nov 16, 2023

oletf commented Nov 16, 2023

Readon Nov 16, 2023

Choose a reason for hiding this comment

oletf Nov 16, 2023 • edited

Choose a reason for hiding this comment

Readon Nov 16, 2023

Choose a reason for hiding this comment

oletf Nov 16, 2023 • edited

Choose a reason for hiding this comment

Readon Nov 16, 2023

Choose a reason for hiding this comment

oletf Nov 16, 2023

Choose a reason for hiding this comment

Readon commented Nov 16, 2023

andreasWallner commented Nov 19, 2023

Readon commented Nov 19, 2023

andreasWallner commented Dec 10, 2023 • edited

Readon commented Dec 11, 2023

Readon commented Dec 11, 2023

Readon commented Dec 11, 2023

Readon commented Dec 11, 2023

oletf commented Dec 11, 2023

oletf commented Dec 11, 2023

Readon commented Dec 11, 2023

Readon commented Dec 11, 2023

Readon commented Dec 31, 2023

Readon commented Jan 22, 2024

oletf Nov 16, 2023 •

edited

oletf Nov 16, 2023 •

edited

andreasWallner commented Dec 10, 2023 •

edited