Add project for LLVM backend (supports Wasm) #247

yowl · 2020-10-19T13:10:44Z

No description provided.

…in64

yowl · 2020-11-25T19:46:03Z

src/libraries/Directory.Build.props

-    <ToolRuntimeRID Condition="'$(RuntimeOS)' == 'browser' and '$(TargetOS)' != 'windows' and !$(_buildingInOSX)">linux-x64</ToolRuntimeRID>
-
+<!-- this line seems to cause a failure with _SuppressSdkImports .  Doesn't make sense to me -->
+<!--    <ToolRuntimeRID Condition="'$(RuntimeOS)' == 'browser' and '$(TargetOS)' != 'windows' and !$(_buildingInOSX)" and !$(_buildingInWindows)">linux-x64</ToolRuntimeRID> -->


I don't understand this. The condition shouldn't even be met as buildingInWindows is true so why it causes an error I don't know.

I guess it may a problem with evaluation order.

Merging the latest got rid of this

yowl · 2020-11-25T19:49:48Z

I'm going to release this now for review. Sorry it's rather long. If you want to know what each change fixes I can revert it and show the error. It will publish and run the Samples\HelloWorld (with https://github.com/dotnet/runtimelab/pull/247/files#diff-95c7fc3099f2a4f95cd78e2aae618395f1a7aef4e88e34d30d957f4814bf688e) and that's as far as I've taken it. Once it goes in I could either:

Add yml for CI
Get the old tests passing - HelloWasm/Exceptions/SimpleGenerics

jkotas · 2020-11-25T19:59:17Z

eng/native/build-commons.ps1

@@ -0,0 +1,21 @@
+if ( -not (Test-Path -LiteralPath $Env:__versionSourceFile))


What's wrong with the default version.c/.h generation that you had to duplicate it?

Deleted. Whatever reason I had has gone.

jkotas · 2020-11-25T20:02:04Z

eng/native/configurecompiler.cmake

+  # The -fms-extensions enable the stuff like __if_exists, __declspec(uuid()), etc.
+  add_compile_options(-fms-extensions)
+  add_compile_options(-Wno-invalid-offsetof)
+  add_compile_options(-Wno-tautological-undefined-compare) # this == NULL warning suppression


I believe that we have cleaned up all these in dotnet/runtime. Are these warning suppressions and -fms-extension really required?

ms-extensions I believe is preventing

In file included from E:\GitHub\runtimelab\src\coreclr\src\nativeaot\Runtime\allocheap.cpp:9: E:/GitHub/runtimelab/src/coreclr/src/nativeaot/Runtime/./PalRedhawk.h:658:1: error: unknown type name '__forceinline' __forceinline ^ In file included from E:\GitHub\runtimelab\src\coreclr\src\nativeaot\Runtime\allocheap.cpp:9: In file included from E:/GitHub/runtimelab/src/coreclr/src/nativeaot/Runtime/./PalRedhawk.h:873:

-Wno-invalid-offsetof is stopping:

In file included from E:\GitHub\runtimelab\src\coreclr\src\nativeaot\Runtime\allocheap.cpp:21: E:/GitHub/runtimelab/src/coreclr/src/nativeaot/Runtime/./slist.inl:41:54: warning: offset of on non-standard-layout type 'AllocHeap::BlockListElem' [-Winvalid-offsetof] return dac_cast<PTR_PTR_T>(dac_cast<TADDR>(pT) + offsetof(T, m_pNext)); ^ ~~~~~~~ E:\GitHub\emsdk\upstream\emscripten\system\include\libc\stddef.h:20:32: note: expanded from macro 'offsetof' #define offsetof(type, member) __builtin_offsetof(type, member) ^ ~~~~~~ E:/GitHub/runtimelab/src/coreclr/src/nativeaot/Runtime/./slist.inl:245:14: note: in instantiation of member function 'DefaultSListTraits<AllocHeap::BlockListElem, DoNothingFailFastPolicy>::GetNextPtr' requested here *Traits::GetNextPtr(pItem) = *m_ppCur; ^ E:/GitHub/runtimelab/src/coreclr/src/nativeaot/Runtime/./slist.inl:85:13: note: in instantiation of member function 'SList<AllocHeap::BlockListElem, DefaultSListTraits<AllocHeap::BlockListElem, DoNothingFailFastPolicy>>::Iterator::Insert' requested here Begin().Insert(pItem); ^ E:\GitHub\runtimelab\src\coreclr\src\nativeaot\Runtime\allocheap.cpp:100:17: note: in instantiation of member function 'SList<AllocHeap::BlockListElem, DefaultSListTraits<AllocHeap::BlockListElem, DoNothingFailFastPolicy>>::PushHead' requested here m_blockList.PushHead(pBlock); ^ 1 warning generated.

-Wno-tautological-undefined-compare is for:

E:\GitHub\runtimelab\src\coreclr\src\nativeaot\Runtime\eetype.cpp:24:9: warning: 'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to false [-Wtautological-undefined-compare] if (this == NULL) ^~~~ ~~~~ 1 warning generated.

I see. These and warning options are in this file under CLR_CMAKE_HOST_UNIX. Would it make sense to change the condition on that block to !MSVC or something similar instead of duplicating them here?

Thanks, moved as suggested to not duplicate.

jkotas · 2020-11-25T20:03:30Z

eng/native/gen-buildsys.cmd

@@ -24,12 +24,13 @@ set __Arch=%4
 set __CmakeGenerator=Visual Studio
 set __UseEmcmake=0
 if /i "%__Ninja%" == "1" (
+	echo using Ninja


Are these echo statements needed? If yes, the alignment is off for this one.

Thanks, missed this one, deleted.

src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems

src/libraries/Native/build-native.cmd

jkotas · 2020-11-25T21:00:22Z

src/libraries/Directory.Build.props

-    <ToolRuntimeRID Condition="'$(RuntimeOS)' == 'browser' and '$(TargetOS)' != 'windows' and !$(_buildingInOSX)">linux-x64</ToolRuntimeRID>
-
+<!-- this line seems to cause a failure with _SuppressSdkImports .  Doesn't make sense to me -->
+<!--    <ToolRuntimeRID Condition="'$(RuntimeOS)' == 'browser' and '$(TargetOS)' != 'windows' and !$(_buildingInOSX)" and !$(_buildingInWindows)">linux-x64</ToolRuntimeRID> -->


I guess it may a problem with evaluation order.

jkotas · 2020-11-25T21:03:11Z

Could you please also update pre-release label as described in https://github.com/dotnet/runtimelab/blob/master/CreateAnExperiment.md so that the packages do not clash?

Merge remote-tracking branch 'origin/feature/NativeAOT-LLVM' into llvm-rename # Conflicts: # src/coreclr/dir.common.props # src/libraries/Directory.Build.props

jkotas · 2020-11-28T16:02:03Z

src/libraries/Directory.Build.props

@@ -256,4 +256,7 @@
    <CLSCompliant Condition="'$(CLSCompliant)' == ''">true</CLSCompliant>
  </PropertyGroup>

+  <PropertyGroup Condition="$([MSBuild]::IsOSPlatform('windows')) and '$(TargetOS)' == 'Browser'"> <!-- the Unix versions are used without this override -->


Is the actual problem a bug in eng\Configurations.props?

I think the block in Configurations.props should be something like:

 <_toolRuntimeRID Condition="'$(_runtimeOS)' == 'browser'">linux-x64</_toolRuntimeRID> <_toolRuntimeRID Condition="'$(_runtimeOS)' == 'browser' and $([MSBuild]::IsOSPlatform('WINDOWS'))">win-x64</_toolRuntimeRID> <_toolRuntimeRID Condition="'$(_runtimeOS)' == 'browser' and $([MSBuild]::IsOSPlatform('OSX'))">osx-x64</_toolRuntimeRID>

Thanks, that looks better. Admittedly I don't know my way around the build files.

This condition should be using the OS that the build is running on.

TargetOS should mean the OS we are compiling for, everywhere in the build system. If TargetOS means the OS that the build is running in some places, I think we should fix those place.

mono's lead on making 'is browser' decision

I am not sure what this means.

TargetOS is not browser on mono when we are building for wasm today

Is it really the case? There are number of places under libraries that compare TargetOS with Browser, one example from many: https://github.com/dotnet/runtime/blob/master/src/libraries/System.Diagnostics.StackTrace/tests/System.Diagnostics.StackTrace.Tests.csproj#L16

If the TargetOS was not set correct for Browser, all these places would be broken.

In any case, I think it is a good idea to keep fixing places that use wrong meaning of TargetOS.

jkotas · 2020-11-28T16:19:56Z

eng/Versions.props

@@ -6,7 +6,7 @@
    <MajorVersion>6</MajorVersion>
    <MinorVersion>0</MinorVersion>
    <PatchVersion>0</PatchVersion>
-    <PreReleaseVersionLabel>alpha</PreReleaseVersionLabel>
+    <PreReleaseVersionLabel>llvm</PreReleaseVersionLabel>


I have realized that this won't be sufficient to avoid collisions given how nuget works and how things are setup currently.

The problem is what PackageReference Include="Microsoft.DotNet.ILCompiler" Version="6.0.0-*" package reference is going to pick up the latest * that will be llvm once this starts publishing.

I think it will be best to use different package name for this branch, like Microsoft.DotNet.ILCompiler.LLVM.

Could you please rename Microsoft.DotNet.ILCompiler to Microsoft.DotNet.ILCompiler.LLVM under src\installer\pkg\projects ? (Both directory names and references in sources.)

There may be a few other places througout the tree that may need similar rename.

Sure, is the label change now superfluous ?

Yes - if you change the package name, it is not necessary to change the label.

Maybe I've gone overboard on this, I renamed the targets file and the pkgproj.

jkotas · 2020-11-28T19:00:58Z

eng/Configurations.props

-    <_toolRuntimeRID Condition="'$(_runtimeOS)' == 'browser' and '$(TargetOS)' == 'windows'">win-x64</_toolRuntimeRID>
-    <_toolRuntimeRID Condition="'$(_runtimeOS)' == 'browser' and '$(TargetOS)' != 'windows' and $(_buildingInOSX)">osx-x64</_toolRuntimeRID>
-    <_toolRuntimeRID Condition="'$(_runtimeOS)' == 'browser' and '$(TargetOS)' != 'windows' and !$(_buildingInOSX)">linux-x64</_toolRuntimeRID>
+    <_toolRuntimeRID Condition="'$(_runtimeOS)' == 'browser'">linux-x64</_toolRuntimeRID>


If it works, it would be nice to submit this to dotnet/runtime. Also, the Android block below should get the same fix.

jkotas · 2020-11-28T19:01:37Z

src/libraries/Native/build-native.cmd

@@ -158,7 +158,8 @@ set __generatorArgs=
 if [%__Ninja%] == [1] (
    set __generatorArgs=
 ) else if [%__BuildArch%] == [wasm] (
-    set __generatorArgs=-j
+    rem nmake does support -j


I think this can also be done in dotnet/runtime

jkotas

LGTM. Thank you!

yowl · 2020-11-29T16:20:28Z

Thanks for all the help.

yowl mentioned this pull request Oct 26, 2020

NativeAOT-LLVM: Question: Platform and arch #260

Closed

jkotas added the area-NativeAOT-LLVM LLVM generation for Native AOT compilation (including Web Assembly) label Oct 26, 2020

EgorBo mentioned this pull request Nov 7, 2020

LLVM as a back-end for RyuJIT #313

Open

wasm/llvm backend to state where Samples\HelloWorld will publish on W…

82dd290

…in64

yowl force-pushed the llvm-rename branch from 2ade99b to 82dd290 Compare November 25, 2020 19:43

yowl marked this pull request as ready for review November 25, 2020 19:43

yowl commented Nov 25, 2020

View reviewed changes

yowl changed the title ~~WIP: add project for LLVM backend (supports Wasm)~~ Add project for LLVM backend (supports Wasm) Nov 25, 2020

jkotas approved these changes Nov 25, 2020

View reviewed changes

yowl mentioned this pull request Nov 26, 2020

Fix path to Unix folder for wasm dotnet/runtime#45261

Merged

yowl added 2 commits November 27, 2020 07:21

remove version creation, add doc description, add version label

c392008

address feedback, partly by:

ced80dc

Merge remote-tracking branch 'origin/feature/NativeAOT-LLVM' into llvm-rename # Conflicts: # src/coreclr/dir.common.props # src/libraries/Directory.Build.props

jkotas reviewed Nov 28, 2020

View reviewed changes

move RID logic

ee51881

jkotas reviewed Nov 28, 2020

View reviewed changes

yowl added 3 commits November 28, 2020 17:00

use a unique package name

6427127

comment typo

fb67d8e

delete comment as its a bit meaningless without checking the history

322df58

jkotas approved these changes Nov 29, 2020

View reviewed changes

jkotas merged commit 150c6f7 into dotnet:feature/NativeAOT-LLVM Nov 29, 2020

yowl deleted the llvm-rename branch November 29, 2020 16:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add project for LLVM backend (supports Wasm) #247

Add project for LLVM backend (supports Wasm) #247

yowl commented Oct 19, 2020

yowl Nov 25, 2020

jkotas Nov 25, 2020

yowl Nov 28, 2020

yowl commented Nov 25, 2020

jkotas Nov 25, 2020

yowl Nov 28, 2020

jkotas Nov 25, 2020

yowl Nov 26, 2020

jkotas Nov 26, 2020

yowl Nov 28, 2020 •

edited

Loading

jkotas Nov 25, 2020

yowl Nov 28, 2020

jkotas Nov 25, 2020

jkotas commented Nov 25, 2020

jkotas Nov 28, 2020

yowl Nov 28, 2020

jkotas Nov 28, 2020

jkotas Nov 28, 2020

jkotas Nov 28, 2020

yowl Nov 28, 2020

jkotas Nov 28, 2020

yowl Nov 28, 2020

jkotas Nov 28, 2020

jkotas Nov 28, 2020

jkotas left a comment

yowl commented Nov 29, 2020

		@@ -0,0 +1,21 @@
		if ( -not (Test-Path -LiteralPath $Env:__versionSourceFile))

Add project for LLVM backend (supports Wasm) #247

Add project for LLVM backend (supports Wasm) #247

Conversation

yowl commented Oct 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yowl commented Nov 25, 2020

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 Nov 28, 2020 • edited Loading

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

jkotas commented Nov 25, 2020

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

jkotas left a comment

Choose a reason for hiding this comment

yowl commented Nov 29, 2020

yowl Nov 28, 2020 •

edited

Loading