Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NativeAOT-LLVM] Merge apr 24 #2543

Merged
merged 1,095 commits into from
Apr 23, 2024
Merged

Conversation

yowl
Copy link
Contributor

@yowl yowl commented Apr 6, 2024

Merges to commit 9b57a26

Of note:

  • Add back SetLayout to GenTreeBlk
  • Add new CorInfo methods, CORINFO_HELP_MEMZERO and CORINFO_HELP_NATIVE_MEMSET
  • Remove GT_STORE_DYN_BLK
  • change javascript files to come from LibrariesNativeArtifactsPath not CoreCLRNativeArtifactsPath in the nuspec

jkotas and others added 30 commits March 25, 2024 10:29
* Delete RequiresProcessIsolation on some tests

- CoreMangLib/system/threading/interlocked/*
- Loader/classloader/generics/*
- baseservices/exceptions/*

* Delete duplicated test. Loader\classloader\generics\regressions\vsw524571\staticsproblem5.cs was a duplicate of Loader\classloader\generics\Statics\Regressions\524571\StaticsProblem5.cs
…138)

This adds a uniform representation that can represent the ABI
information for all of our targets without needing to fall back to
handling ABI specific details in all places that need to handle calling
conventions.

Currently nothing is using this information. I want to incrementally
migrate our ABI handling to use this representation. Also, there are
several potential future improvements:

- Split out ABI classification per ABI instead of keeping them all
  within the same function
- Unify `InitVarDscInfo::stackArgSize` and `InitVarDscInfo::argSize`. I
  am unsure why the latter is needed
- Remove `LclVarDsc::GetArgReg()`, `LclVarDscInfo::GetOtherArgReg()`,
  HFA related members
- Reuse the representation in `CallArgABIInformation` and unify the
  classifiers

The end goal here is rewriting `genFnPrologCalleeRegArgs` to handle
float and integer registers simultaneously, and to support some of the
registers that the Swift calling convention is using.
* Fix HalfNumberBufferLength

* Add regression test
* Update dependencies from https://github.com/dotnet/runtime build

Microsoft.DotNet.ILCompiler , Microsoft.NET.Sdk.IL , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.ILAsm , runtime.native.System.IO.Ports , System.Reflection.Metadata , System.Reflection.MetadataLoadContext , System.Text.Json , Microsoft.SourceBuild.Intermediate.runtime.linux-x64
 From Version 9.0.0-preview.3.24165.13 -> To Version 9.0.0-preview.4.24175.1

* Update dependencies from https://github.com/dotnet/sdk build

Microsoft.SourceBuild.Intermediate.sdk , Microsoft.DotNet.ApiCompat.Task
 From Version 9.0.100-preview.3.24168.1 -> To Version 9.0.100-preview.4.24175.4

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
…(#100173)

* Populate JsonPropertyInfo.AttributeProvider in the source generator.

* Add test assertion checking the property type.
…calls (#100152)

* Set bbCodeOffsEnd to BAD_IL_OFFSET when expanding static init calls

* Re-enable NativeAOT tests that were failing
Without this fix, it was possible for another thread to see an incorrect
(zero) value of s_timeBase_numer because of a race condition.
* Provide Threading.Tasks.Dataflow package readme

* Processed review comments
- Switch some `IsMultiRegReturnedType` calls to use
`GenTreeCall::HasMultiRegRetVal`.
- Remove `Compiler*` parameter of `TreatAsShouldHaveRetBufArg`; the
  function it was using on `Compiler` is static
The operation of `mkrefany` can easily be represented with more
generally handled nodes within the JIT today. This also allows promotion
to remain enabled for methods using this construct, so CQ improvements
are expected when optimizing.
* Fix NativeName and DisplayName for browser.

* Nit - revert over-renaming.

* Update src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs

Co-authored-by: Meri Khamoyan <96171496+mkhamoyan@users.noreply.github.com>

* Enable more tests and fix them - initialize english and native names on CultureInfo constructio

* Ubnblock fixed test.

* MT does not work with HG well, revet MT to original way of working.

* Expect fixed version of NativeName in tests with single treaded runtime.

* Assert.Contains does not know that `\u00F1` is same as `ñ` (it does not evaluate the string before comparison) - fix it with string interpolation.

* Windows has problems with comparing utf8 by xunit.

---------

Co-authored-by: Meri Khamoyan <96171496+mkhamoyan@users.noreply.github.com>
…nals build (#100285)

Microsoft.SourceBuild.Intermediate.source-build-externals
 From Version 9.0.0-alpha.1.24168.3 -> To Version 9.0.0-alpha.1.24175.4

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Implements a Gauss-Seidel solver for cases where method have irreducible loops.
…vers (#99909)

* Send the NegotiateSeal NTLM flag when client asked for
ProtectionLevel.EncryptAndSign.

Process the last handshake done message in NegotiateStream. In case of
SPNEGO protocol it may contain message integrity check. Additionally,
if the negotiated protocol is NTLM then we need to reset the encryption
key after the message integrity check is verified.

* Add test for the NegotiateSeal flag

* Fix the test

* Dummy commit

* Fix the new _remoteOk logic in NegotiateStream to fire only when HandshakeComplete.

If HandshakeComplete is not true, then the authentication blob will get processed with the normal flow.

* Fix the value of NegotiateSeal in the final authentication message of Managed NTLM
Since build-in COM marshalling relies on the declaration
order to build the RCW vtable, this is and has almost always
resulted in an A/V or undefined behavior.
If a node has an order side effect, we can't hoist it at all:
we don't know what the order dependence actually is. For example,
assertion prop might have determined a node can't throw an exception,
and eliminated the `GTF_EXCEPT` flag, replacing it with `GTF_ORDER_SIDEEFF`.
We can't hoist because we might then hoist above the expression that
led assertion prop to make that decision. This can happen in JitOptRepeat,
where hoisting can follow assertion prop.
Move a check from `CallArgs::GetCustomRegister` to `hasFixedRetBuffReg`.
Returning `TYP_INT` from this function was causing `lvaInitUserArgs` to
not set the "other reg" in `LclVarDsc` for SIMD types in varargs
methods. This seemingly didn't usually cause issues, probably because we
always DNER them and apparently `genFnPrologCalleeRegArgs` does not use
`LclVarDsc::GetOtherArgReg()` in this case.
* [RISC-V] Fixes

* [RISC-V] Final fixes

* [RISC-V] Fixed NBitMask issue

* [RISC-V] Removed some of the redundancy in the NBitMask

* [RISC-V] Renamed NBitMask to WordMask and simplified it's implementation

* [RISC-V] Removed noexcept from WordMask
remove DotnetJs test
@@ -114,6 +115,7 @@ class RegSet
assert(rsModifiedRegsMaskInitialized);
return (rsModifiedRegsMask & RBM_FLT_CALLEE_SAVED);
}
#endif // !TARGET_WASM

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#endif // !TARGET_WASM
Suggested change
#endif // !TARGET_WASM

And add RBM_FLT_CALLEE_SAVED to targetwasm.h.

@@ -119,4 +119,7 @@

#define RBM_ARG_REGS RBM_R0
#define RBM_FLTARG_REGS RBM_F0
// clang-format on

#define RBM_INT_CALLEE_SAVED RBM_NONE;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src/coreclr/jit/llvmcodegen.cpp Outdated Show resolved Hide resolved
eng/pipelines/runtimelab.yml Outdated Show resolved Hide resolved
eng/pipelines/runtimelab.yml Outdated Show resolved Hide resolved
eng/pipelines/runtimelab.yml Outdated Show resolved Hide resolved
postBuildSteps:
- template: /eng/pipelines/runtimelab/runtimelab-post-build-steps.yml
parameters:
librariesConfiguration: Release

- ${{ else }}:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this else branch for the official build, I think we want to (once again) delete the Build libraries AllConfigurations for packages job and tweak the Build the whole product with Release CoreCLR one to also include our post-steps and build nativeaot.packages (basically make it look like the other ones).

@@ -15,42 +14,52 @@ import {
get_sig, get_signature_argument_count,
bound_cs_function_symbol, get_signature_version, alloc_stack_frame, get_signature_type,
} from "./marshal";
import { utf16ToString } from "./strings";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the import move?

yowl and others added 10 commits April 23, 2024 12:50
…aded.cpp

Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
…stem.Runtime.InteropServices.JavaScript.csproj

Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
Comment on lines 135 to 138
# Upload the results.
- template: /eng/pipelines/common/upload-intermediate-artifacts-step.yml
parameters:
name: $(osGroup)$(osSubgroup)_$(archType)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Upload the results.
- template: /eng/pipelines/common/upload-intermediate-artifacts-step.yml
parameters:
name: $(osGroup)$(osSubgroup)_$(archType)

runtimelab-post-build-steps.yml also does this step. I suspect doing it twice will not lead to good things.

Comment on lines 144 to 147
${{ if eq(variables.isOfficialBuild, false) }}:
buildArgs: -s clr.aot+libs+nativeaot.packages -c release /p:ArchiveTests=true
${{ if eq(variables.isOfficialBuild, true) }}:
buildArgs: -s clr.aot+libs+nativeaot.packages -c release

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I don't understand this. Shouldn't this be above, i. e. buildArgs: -s clr+libs+hosts+packs -c $(_BuildConfig) -> buildArgs: -s clr.aot+libs+nativeaot.packages -c $(_BuildConfig)?

@lewing
Copy link
Member

lewing commented Apr 23, 2024

@agocke can you help with some of the pipeline questions

Copy link

@SingleAccretion SingleAccretion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given green CI (I think that means disabling the JS test again), LGTM.

Thank you for all the work on this!

This reverts commit 53cd4b8.
@yowl
Copy link
Contributor Author

yowl commented Apr 23, 2024

Thank you for all the work on this!

Thank you for your time and help in the review.

@jkotas
Copy link
Member

jkotas commented Apr 23, 2024

Thank you all!

@jkotas jkotas merged commit 4a2148f into dotnet:feature/NativeAOT-LLVM Apr 23, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-NativeAOT-LLVM LLVM generation for Native AOT compilation (including Web Assembly)
Projects
None yet
Development

Successfully merging this pull request may close these issues.