[NativeAOT-LLVM] Build on Linux #2574

yowl · 2024-05-05T12:44:27Z

This PR reenables the build of the product on Linux, including the LLVM JIT. Does not build the Wasm runtime or libraries on Linux (to be done in #2569 )

Fixes #1713

jkotas · 2024-05-05T16:40:54Z

src/mono/browser/runtime/package-lock.json

@@ -1,8 +1,2756 @@
 {
  "name": "@microsoft/dotnet-runtime",


Are the changes in this file intentional - why do we need them?

No, reverted.

yowl · 2024-05-05T23:25:36Z

cc @dotnet/nativeaot-llvm

workflow

SingleAccretion · 2024-05-06T11:41:15Z

eng/native/gen-buildsys.sh

+        if [[ -n "$NATIVEAOT_CI_WASM_BUILD_EMSDK_PATH" ]]; then
+             source $NATIVEAOT_CI_WASM_BUILD_EMSDK_PATH/emsdk_env.sh
+        fi


The Windows equivalent is in eng/common/build.ps1. Can we do the same with eng/common/build.sh to keep the scripts similar?

Yes, moved.

SingleAccretion · 2024-05-06T11:43:08Z

eng/pipelines/common/global-build-job.yml

+      # Install Wasm dependencies on Linux: emscripten, LLVM, NodeJS.
+    - ${{ if and(eq(parameters.hostedOs, 'linux'), and(eq(parameters.runtimeFlavor, 'coreclr'), eq(parameters.archType, 'wasm'))) }}:
+      - script: $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-emscripten.sh $(Build.SourcesDirectory)/wasm-tools
+        displayName: Install/activate emscripten
+      - script: pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-llvm.ps1 -Configs ${{ parameters.buildConfig }} -CI
+        workingDirectory: $(Build.SourcesDirectory)/wasm-tools
+        displayName: Install/build LLVM
+      - script: pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-nodejs.ps1 $(Build.SourcesDirectory)/wasm-tools
+        displayName: Install NodeJS


Would it work to unify the both Windows and Linux on pwsh?

yes, removed the .cmd

SingleAccretion · 2024-05-06T11:43:54Z

eng/pipelines/common/global-build-job.yml

+    - ${{ if and(eq(parameters.hostedOs, 'linux'), and(eq(parameters.runtimeFlavor, 'coreclr'), eq(parameters.archType, 'wasm'))) }}:
+      - script: $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-emscripten.sh $(Build.SourcesDirectory)/wasm-tools
+        displayName: Install/activate emscripten
+      - script: pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-llvm.ps1 -Configs ${{ parameters.buildConfig }} -CI


parameters.buildConfig can be Checked, to the normalization of of Config in install-llvm.cmd needs to be moved into install-llvm.ps1.

SingleAccretion · 2024-05-06T11:46:20Z

eng/pipelines/runtimelab.yml

@@ -59,7 +59,7 @@ extends:
            helixQueuesTemplate: /eng/pipelines/coreclr/templates/helix-queues-setup.yml
            buildConfig: debug
            platforms:
-#            - linux_x64
+            - linux_x64


These linux_x64 jobs only test upstream code - I think we can do with just one to exercise our changes to the build scripts (say, only Release).

RIght, have left just Release.

SingleAccretion · 2024-05-06T11:47:02Z

eng/pipelines/runtimelab/install-emscripten.sh

@@ -0,0 +1,16 @@
+#!/bin/bash


Perhaps we can use the same powershell sharing strategy as with the the other scripts?

right, have deletes the .cmd/.sh and just left the ps1

SingleAccretion · 2024-05-06T11:52:26Z

eng/pipelines/runtimelab/install-nodejs.ps1

+    Expand-Archive -LiteralPath "$InstallPath\$NodeJSZipName" -DestinationPath $InstallPath -Force
+    $NodeJSExePath = "$InstallPath\$NodeJSInstallName\node.exe"


Does Expand-Archive not work on Linux (it's a bit surprising)?

It does, but doesn't support gz which is gzip, or xz which is some LZMA I think.

And those are our options for linux https://nodejs.org/dist/v9.9.0/

SingleAccretion · 2024-05-06T11:53:09Z

src/coreclr/build-runtime.sh

@@ -171,6 +171,26 @@ if [[ -n "$__RequestedBuildComponents" ]]; then
    __CMakeTarget=" $__RequestedBuildComponents "
    __CMakeTarget="${__CMakeTarget// paltests / paltests_install }"
 fi
+
+if [[ "$__CMakeTarget" == *"wasmjit"* ]]; then
+  __ExtraCmakeArgs="$__ExtraCmakeArgs -DCLR_CMAKE_BUILD_LLVM_JIT=1"


Do we need __ExtraCmakeArgs in addition to __CMakeArgs?

missed this one, but is in now.

SingleAccretion · 2024-05-06T11:54:05Z

src/coreclr/jit/CMakeLists.txt

+    find_package(LLVM REQUIRED CONFIG PATHS $ENV{LLVM_CMAKE_CONFIG} NO_DEFAULT_PATH)
+    find_package(LLVM REQUIRED CONFIG)


Does this mean we will search for the system LLVM install too? That would not be correct.

It does, removed.

SingleAccretion · 2024-05-06T11:55:38Z

src/native/libs/build-native.sh

@@ -60,15 +60,15 @@ source "$__RepoRootDir"/eng/native/build-commons.sh

 # Set cross build
 if [[ "$__TargetOS" == browser ]]; then
-    if [[ -z "$EMSDK_PATH" ]]; then
+    if [[ -z "$EMSDK" ]]; then


It would be better to just set EMSDK_PATH to EMSDK somewhere above s.t. these lines do not need to be modified.

Done, thanks

SingleAccretion · 2024-05-06T11:56:54Z

src/tests/Common/tests.targets

@@ -6,7 +6,7 @@
  </ItemGroup>

  <PropertyGroup>
-      <TestAssemblyDir Condition="'$(TestAssemblyDir)' == ''">$(BaseOutputPathWithConfig)\tests\</TestAssemblyDir>
+      <TestAssemblyDir Condition="'$(TestAssemblyDir)' == ''">$(BaseOutputPathWithConfig)\Tests\</TestAssemblyDir>


Why does upstream not run into the same problem (in other words - is this an upstream bug)?

Have reverted this one, it looked wrong, but wasn't the problem I thought it was.

yowl · 2024-05-10T23:58:37Z

Lots of little commits, but think that's all the comments above addressed.

SingleAccretion · 2024-05-10T18:16:42Z

eng/common/build.sh

+host_arch=''
+target_os=''


Are these only required for NATIVEAOT_CI_WASM_BUILD_EMSDK_PATH? It's not clear what they're for.

Its to avoid

/home/scott/github/runtimelab/eng/common/build.sh: line 245: host_arch: unbound variable

${host_arch-''} might be another way to solve it, or make it a mandatory parameter.

But the only uses of host_arch that I see look to be new (added in this change). My suggestion is delete them - or are they needed for something?

SingleAccretion · 2024-05-10T18:22:05Z

eng/pipelines/runtimelab/install-emscripten.ps1

+python ./emsdk.py install 3.1.47
+
+./emsdk activate 3.1.47


We have install-emscripten.cmd referenced in docs/using-nativeaot/prerequisites.md and also docs/workflow/building/coreclr/nativeaot.md, these need to be updated now.

Also, it is odd that the first line calls .py directly while the second goes through the wrapper script.

src/coreclr/build-runtime.sh

src/coreclr/jit/CMakeLists.txt

SingleAccretion · 2024-05-11T13:54:08Z

eng/pipelines/runtimelab/install-emscripten.ps1

+    [string]$InstallDir
+)
+
+New-Item -ItemType Directory -Force -ErrorAction SilentlyContinue -Path (Split-Path -Path $InstallDir -Parent) -Name (Split-Path -Path $InstallDir -Leaf)


Does this really need to be this elaborate?

I just tried:

/home/singleaccretion> New-Item /home/singleaccretion/test/xyz/dir -Force -ItemType Directory

And it created the nested directory as expected.

Its ok on Linux, but with a drive letter it fails. We could strip the drive letter and assume we are "in" that drive, but it probably isn't much easier to read.

PS C:\Users\scott> new-item -ItemType Directory -Force -Name c:/tmp/1/2 new-item : The given path's format is not supported. At line:1 char:1 + new-item -ItemType Directory -Force -Name c:/tmp/1/2 + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + CategoryInfo : NotSpecified: (:) [New-Item], NotSupportedException + FullyQualifiedErrorId : System.NotSupportedException,Microsoft.PowerShell.Commands.NewItemCommand PS C:\Users\scott> new-item -ItemType Directory -Force -Name /tmp/1/2 Directory: C:\Users\scott\tmp\1 Mode LastWriteTime Length Name ---- ------------- ------ ---- d----- 11/05/2024 16:54 2

new-item -ItemType Directory -Force -Name c:/tmp/1/2

I think this should use -Path. I tried:

PS C:\Users\Accretion\AppData\Local\Temp> new-item -Force -ItemKind Directory -Path C:\Users\Accretion\AppData\Local\Temp/xyz/rot/tot Directory: C:\Users\Accretion\AppData\Local\Temp\xyz\rot Mode LastWriteTime Length Name ---- ------------- ------ ---- d---- 13.05.2024 00:22 tot

SingleAccretion · 2024-05-11T13:55:50Z

eng/pipelines/common/global-build-job.yml

+        displayName: Install NodeJS
+
+      # Install Wasm dependencies on Linux: emscripten, LLVM, NodeJS.
+    - ${{ if and(eq(parameters.hostedOs, 'linux'), and(eq(parameters.runtimeFlavor, 'coreclr'), eq(parameters.archType, 'wasm'))) }}:


Can we collapse the Windows/Linux into one section now (will require putting Emscripten on the same pwsh plan)?

Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>

SingleAccretion · 2024-05-11T17:16:08Z

eng/pipelines/runtimelab/install-emscripten.ps1

+New-Item -ItemType Directory -Force -ErrorAction SilentlyContinue -Name $InstallDir
+
+$ErrorActionPreference="Stop"
+
+Set-Location -Path $InstallDir


Suggested change

New-Item -ItemType Directory -Force -ErrorAction SilentlyContinue -Name $InstallDir

$ErrorActionPreference="Stop"

Set-Location -Path $InstallDir

$ErrorActionPreference="Stop"

New-Item -Path $InstallDir -ItemType Directory -Force

Set-Location -Path $InstallDir

I think this ought to work...

Yes, indeed that does.

SingleAccretion · 2024-05-11T17:22:46Z

eng/pipelines/common/global-build-job.yml

        displayName: Install/activate emscripten
-      - script: call $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-llvm.cmd $(Build.SourcesDirectory)\wasm-tools $(Build.SourcesDirectory) ${{ parameters.buildConfig }}
+      - script: pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-llvm.ps1 $(Build.SourcesDirectory)\wasm-tools $(Build.SourcesDirectory) ${{ parameters.buildConfig }}


Suggested change

- script: pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-llvm.ps1 $(Build.SourcesDirectory)\wasm-tools $(Build.SourcesDirectory) ${{ parameters.buildConfig }}

- script: pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-llvm.ps1 $(Build.SourcesDirectory)/wasm-tools $(Build.SourcesDirectory) -Configs ${{ parameters.buildConfig }} -CI

The cmake detection and cd $(Build.SourcesDirectory)/wasm-tools need to be migrated from install-llvm.cmd to install-llvm.ps1.

The cmake detection, i.e. executing init-vs-env.cmd as we do, doesn't seem necessary, and puts the CMakePath at the end anyway so doesn't precede anything already in the path. It is possible to do from powershell, but would involve a wrapper cmd to echo the CMakePath back to the powershell do you scoping of environment variables and the lack or an equivalent call in powershell. Question is, do we really need it?

I had assumed it was needed. If that's not the case - great!

We do need cmake.exe in the path, so have added this back, which means we need a cmd for the windows side.

SingleAccretion · 2024-05-11T17:25:09Z

eng/common/build.sh

+host_arch=''
+target_os=''


But the only uses of host_arch that I see look to be new (added in this change). My suggestion is delete them - or are they needed for something?

yowl · 2024-05-12T11:13:47Z

"But the only uses of host_arch that I see look to be new (added in this change). My suggestion is delete them - or are they needed for something?"

You are right, I don't actually need it here, but I will for ./build.sh clr.aot -c Debug -a wasm -os browser

SingleAccretion · 2024-05-12T21:27:02Z

My preference would be introduce them in the WASM change. In general, it looks unexpected to me that we need to change eng/common/build.sh, since these scripts are used upstream for -a wasm -os browser builds too.

Remove wasm build options add cmake to path on windows Use more Powershell where possible Simplify New-Item use Feedback

yowl · 2024-05-21T13:47:31Z

My preference would be introduce them in the WASM change. In general, it looks unexpected to me that we need to change eng/common/build.sh, since these scripts are used upstream for -a wasm -os browser builds too.

OK, removed here.

yowl · 2024-05-21T13:48:23Z

Was on a break last week, so did this with lots of trail and error, but have now rebased to remove that noise and is green.

SingleAccretion · 2024-05-21T16:25:14Z

eng/pipelines/common/global-build-job.yml

+    - ${{ if eq(parameters.archType, 'x64') }}:
+      - ${{ if eq(parameters.osGroup, 'windows') }}:
+        - script: call $(Build.SourcesDirectory)/eng/pipelines/runtimelab/set-cmake-path.cmd
+          displayName: Set CMake path
+        - script: pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-llvm.ps1 -CI -InstallDir $(Build.SourcesDirectory)/wasm-tools -Configs ${{ parameters.buildConfig }}
+          displayName: Install/build LLVM
+      - ${{ if eq(parameters.osGroup, 'linux') }}:
+        - script: $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-pwsh.sh $(Build.SourcesDirectory)/wasm-tools
+          displayName: Install Powershell 7
+        # Install LLVM for the win-x64 and linux-x64 build as it will build the clrjit for browser_wasm cross compilation
+        - script: $(Build.SourcesDirectory)/wasm-tools/powershell7/pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-llvm.ps1 -CI -InstallDir $(Build.SourcesDirectory)/wasm-tools -Configs ${{ parameters.buildConfig }}
+          displayName: Install/build LLVM


This will install tools for host configurations (windows_x64, linux_x64) that do not actually target WASM. I think the LLVM stuff should be deleted for windows_x64 and Install Powershell 7 folded into the above block (under {{ if and(eq(parameters.runtimeFlavor, 'coreclr'), eq(parameters.archType, 'wasm')) }}:) under if ne(parameters.hostedOs, 'windows') (this way it will, hopefully, also work for macOS if/when that comes).

Thanks, done. The Linux build isn't pulling in these tools with this change but that's fine, they will be with #2569

SingleAccretion · 2024-05-21T16:26:05Z

eng/pipelines/common/global-build-job.yml

+        - script: $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-pwsh.sh $(Build.SourcesDirectory)/wasm-tools
+          displayName: Install Powershell 7
+        # Install LLVM for the win-x64 and linux-x64 build as it will build the clrjit for browser_wasm cross compilation
+        - script: $(Build.SourcesDirectory)/wasm-tools/powershell7/pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-llvm.ps1 -CI -InstallDir $(Build.SourcesDirectory)/wasm-tools -Configs ${{ parameters.buildConfig }}


install-pwsh.sh sets PATH. Does a plain pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-llvm.ps1 ... not work?

I think that will work, yes. Its not actually exercised with this PR but should be fine.

SingleAccretion · 2024-05-21T20:09:54Z

eng/pipelines/common/global-build-job.yml

        displayName: Install/activate emscripten
-      - script: call $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-llvm.cmd $(Build.SourcesDirectory)\wasm-tools $(Build.SourcesDirectory) ${{ parameters.buildConfig }}
+      - script: call $(Build.SourcesDirectory)/eng/pipelines/runtimelab/set-cmake-path.cmd


I suspect this will need to be put under if eq(parameters.hostedOs, 'windows'), but let's leave that for the next change.

SingleAccretion

LGTM, thank you!

jkotas

Thank you!

yowl added 2 commits May 4, 2024 02:32

Enable linux builds

68c98bc

revert package-lock.json

7f4de79

yowl force-pushed the linux branch 2 times, most recently from 5ca340e to 68c98bc Compare May 5, 2024 15:50

jkotas added the area-NativeAOT-LLVM LLVM generation for Native AOT compilation (including Web Assembly) label May 5, 2024

jkotas reviewed May 5, 2024

View reviewed changes

yowl marked this pull request as ready for review May 5, 2024 23:25

yowl added 3 commits May 6, 2024 04:06

move NATIVEAOT_CI_WASM_BUILD_EMSDK_PATH to be inline with windows

6198eee

workflow

move NATIVEAOT_CI_WASM_BUILD_EMSDK_PATH to be inline with windows

91fa499

just use pwsh for installing node. Delete the .cmd

9ae54c2

SingleAccretion reviewed May 6, 2024

View reviewed changes

yowl added 12 commits May 6, 2024 07:29

normalize checked for llvm config

ef6cdaa

add missing )

b4df762

use powershell only for emscripten install

500a55d

use pwsh inline with node

6d3ca37

add ps1 file

ba2d7af

use New-Item correctly

d08e915

recommendations for installing llvm

65ba66a

do not search system paths for LLVM

bc6f595

feedback tidy

36250d2

do not use __ExtraCmakeArgs

d6e968b

feedback, tidy/consolidate

14724d5

revert to complex directory creation

49a8d0e

SingleAccretion reviewed May 11, 2024

View reviewed changes

yowl and others added 3 commits May 11, 2024 16:47

Update src/coreclr/build-runtime.sh

88f45f8

Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>

Update src/coreclr/jit/CMakeLists.txt

e860a5d

Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>

Merge branch 'linux' of github.com:yowl/runtimelab into linux

f1d8712

SingleAccretion reviewed May 11, 2024

View reviewed changes

Ensure Powershell on linux

e1adb5a

Remove wasm build options add cmake to path on windows Use more Powershell where possible Simplify New-Item use Feedback

yowl force-pushed the linux branch from de08345 to e1adb5a Compare May 21, 2024 10:42

SingleAccretion reviewed May 21, 2024

View reviewed changes

feedback, move tool installations to correct place.

342e14e

SingleAccretion reviewed May 21, 2024

View reviewed changes

SingleAccretion approved these changes May 21, 2024

View reviewed changes

jkotas approved these changes May 21, 2024

View reviewed changes

jkotas merged commit 07ff833 into dotnet:feature/NativeAOT-LLVM May 21, 2024
12 checks passed

yowl deleted the linux branch May 21, 2024 20:57

		Expand-Archive -LiteralPath "$InstallPath\$NodeJSZipName" -DestinationPath $InstallPath -Force
		$NodeJSExePath = "$InstallPath\$NodeJSInstallName\node.exe"

		find_package(LLVM REQUIRED CONFIG PATHS $ENV{LLVM_CMAKE_CONFIG} NO_DEFAULT_PATH)
		find_package(LLVM REQUIRED CONFIG)

	- script: pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-llvm.ps1 $(Build.SourcesDirectory)\wasm-tools $(Build.SourcesDirectory) ${{ parameters.buildConfig }}
	- script: pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-llvm.ps1 $(Build.SourcesDirectory)/wasm-tools $(Build.SourcesDirectory) -Configs ${{ parameters.buildConfig }} -CI

		host_arch=''
		target_os=''

		host_arch=''
		target_os=''

[NativeAOT-LLVM] Build on Linux #2574

[NativeAOT-LLVM] Build on Linux #2574

Conversation

yowl commented May 5, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yowl commented May 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yowl May 10, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yowl commented May 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yowl commented May 12, 2024

SingleAccretion commented May 12, 2024 • edited

yowl commented May 21, 2024

yowl commented May 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SingleAccretion left a comment

Choose a reason for hiding this comment

jkotas left a comment

Choose a reason for hiding this comment

yowl commented May 5, 2024 •

edited

yowl May 10, 2024 •

edited

SingleAccretion commented May 12, 2024 •

edited