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

Add pipeline to run Pri-0 CoreCLR tests #128

Merged
merged 9 commits into from
Oct 2, 2020

Conversation

MichalStrehovsky
Copy link
Member

No description provided.

@MichalStrehovsky MichalStrehovsky added the area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation label Sep 28, 2020
@MichalStrehovsky
Copy link
Member Author

/azp run runtimelab-nativeaot Pri-0 Tests

@azure-pipelines
Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@MichalStrehovsky
Copy link
Member Author

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

Well, anyway. I triggered it at https://dev.azure.com/dnceng/public/_build/results?buildId=833511&view=results

I don't know what the devops voodoo to let me define a pipeline that doesn't exist in the branch yet (I have a pipeline against the pull request branch). Once we merge, I'll try this again.

@jkotas jkotas requested a review from safern September 28, 2020 13:44
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM. I may be good idea for somebody like Santi who understands the intricacies of the CI setup to take a look too.

@MichalStrehovsky
Copy link
Member Author

@safern do you have an opinion on this? I think I mirrored how we do the crossgen2 testing over in the runtime repo (just a separate YAML file, without a cron schedule).

Testing is currently structured the way it is (no Helix, just build and run on the build machine) because the Helix machines don't have stuff that is necessary to do AOT compilation (AFAIK Mono's AOT test solutions suffer from the same problem).

@@ -4,21 +4,26 @@ parameters:
osGroup: ''
osSubgroup: ''
uploadTests: false
smokeTestsOnly: 'true'
Copy link
Member

Choose a reason for hiding this comment

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

no need to escape this string.

@@ -0,0 +1,46 @@
trigger: none

pr: none
Copy link
Member

Choose a reason for hiding this comment

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

why don't want it to be triggered by PR? or CI?

Copy link
Member Author

Choose a reason for hiding this comment

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

It takes 5 hours to complete right now and it's not of a huge value for most changes. Having it on a similar plan as GCStress runs seems ideal. Thanks to the pointer to the issue discussing pr: none. I'll try that out!

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. Then I would recommend just removing the pr: none from here and then setting up the pipeline accordingly in the UI so that it is optional and only triggered by comments.

buildArgs: -s nativeaot+libs+installer -lc release -rc checked
extraStepsTemplate: /eng/pipelines/runtimelab/runtimelab-post-build-steps.yml
extraStepsParameters:
smokeTestsOnly: 'false'
Copy link
Member

Choose a reason for hiding this comment

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

no need to escape this value.

buildArgs: -s nativeaot+libs+installer -c $(_BuildConfig) /p:ArchiveTests=true
extraStepsTemplate: /eng/pipelines/runtimelab/runtimelab-post-build-steps.yml
extraStepsParameters:
smokeTestsOnly: 'false'
Copy link
Member

Choose a reason for hiding this comment

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

no need to escape this.

@safern
Copy link
Member

safern commented Sep 30, 2020

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

This is because of the -pr: none trigger in yml file. Coincidentally we have an issue for some pipelines hitting this on dotnet/runtime. You can see this: dotnet/runtime#32777 (comment) which explains how to enable the ability to run via /azp run but not enable the build by default on all PRs.

I wonder, why do we want to make this pipeline optional for this branch and not run it on all PRs for this branch and CI as well?

@MichalStrehovsky
Copy link
Member Author

/azp run runtimelab-nativeaot Pri-0 Tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MichalStrehovsky MichalStrehovsky merged commit 7c2a5b9 into feature/NativeAOT Oct 2, 2020
@MichalStrehovsky
Copy link
Member Author

@jkotas do you have admin right in this repo? somehow feature/NativeAOT-clrtests became a protected branch. I want to delete it because I won't need it anymore.

@jkotas jkotas deleted the feature/NativeAOT-clrtests branch October 2, 2020 14:16
@jkotas
Copy link
Member

jkotas commented Oct 2, 2020

Done. The branch protection rule is set to feature/*, so everything that starts with feature/ will be protected.

@MichalStrehovsky
Copy link
Member Author

Done. The branch protection rule is set to feature/*, so everything that starts with feature/ will be protected.

Ah, good to know. Thank you!

runtimelab-bot pushed a commit that referenced this pull request Feb 8, 2022
# Local heap optimizations on Arm64

1. When not required to zero the allocated space for local heap (for sizes up to 64 bytes) - do not emit zeroing sequence. Instead do stack probing and adjust stack pointer:

```diff
-            stp     xzr, xzr, [sp,#-16]!
-            stp     xzr, xzr, [sp,#-16]!
-            stp     xzr, xzr, [sp,#-16]!
-            stp     xzr, xzr, [sp,#-16]!
+            ldr     wzr, [sp],#-64
```

2. For sizes less than one `PAGE_SIZE` use `ldr wzr, [sp], #-amount` that does probing at `[sp]` and allocates the space at the same time. This saves one instruction for such local heap allocations:

```diff
-            ldr     wzr, [sp]
-            sub     sp, sp, #208
+            ldr     wzr, [sp],#-208
```

Use `ldp tmpReg, xzr, [sp], #-amount` when the offset not encodable by post-index variant of `ldr`:
```diff
-            ldr     wzr, [sp]
-            sub     sp, sp, #512
+            ldp     x0, xzr, [sp],#-512
```

3. Allow non-loop zeroing (i.e. unrolled sequence) for sizes up to 128 bytes (i.e. up to `LCLHEAP_UNROLL_LIMIT`). This frees up two internal integer registers for such cases:

```diff
-            mov     w11, #128
-                                               ;; bbWeight=0.50 PerfScore 0.25
-G_M44913_IG19:        ; gcrefRegs=00F9 {x0 x3 x4 x5 x6 x7}, byrefRegs=0000 {}, byref, isz
             stp     xzr, xzr, [sp,#-16]!
-            subs    x11, x11, #16
-            bne     G_M44913_IG19
+            stp     xzr, xzr, [sp,#-112]!
+            stp     xzr, xzr, [sp,#16]
+            stp     xzr, xzr, [sp,#32]
+            stp     xzr, xzr, [sp,#48]
+            stp     xzr, xzr, [sp,#64]
+            stp     xzr, xzr, [sp,#80]
+            stp     xzr, xzr, [sp,#96]
```

4. Do zeroing in ascending order of the effective address:

```diff
-            mov     w7, #96
-G_M49279_IG13:
             stp     xzr, xzr, [sp,#-16]!
-            subs    x7, x7, #16
-            bne     G_M49279_IG13
+            stp     xzr, xzr, [sp,#-80]!
+            stp     xzr, xzr, [sp,#16]
+            stp     xzr, xzr, [sp,#32]
+            stp     xzr, xzr, [sp,#48]
+            stp     xzr, xzr, [sp,#64]
```

In the example, the zeroing is done at `[initialSp-16], [initialSp-96], [initialSp-80], [initialSp-64], [initialSp-48], [initialSp-32]` addresses. The idea here is to allow a CPU to detect the sequential `memset` to `0` pattern and switch into write streaming mode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants