SPMI no ASMDiff between Checked and Release by JulieLeeMSFT · Pull Request #61335 · dotnet/runtime

JulieLeeMSFT · 2021-11-09T00:09:33Z

Created the first file that builds release and checked builds

ghost · 2021-11-09T00:09:39Z

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

Created the first file that builds release and checked builds

Author:	JulieLeeMSFT
Assignees:	-
Labels:	`area-Infrastructure-coreclr`
Milestone:	-

JulieLeeMSFT · 2021-11-13T04:07:52Z

Fixes #49949.

ghost · 2022-01-01T23:00:58Z

Draft Pull Request was automatically closed for inactivity. Please let us know if you'd like to reopen it.

JulieLeeMSFT · 2022-01-28T23:51:12Z

@BruceForstall PTAL.
cc @dotnet/jit-contrib

BruceForstall · 2022-01-29T00:25:42Z

Pipeline is https://dev.azure.com/dnceng/public/_build?definitionId=1068

BruceForstall

There is an open question here about whether the existing superpmi asmdiffs pipeline py scripts (and maybe .proj file and .yml files) should be extended with additional flags/options, and the checked/release case should share them, since there is so much duplicated code. That would, of course, make the files more complicated, as they handle more cases.

Another option: this code is focused on checked/release (which is really what we want). A slight generalization is to handle release/release, or even debug/release. Basically, any asm diffs where one or more of the JITs is Release. The key is that a release JIT can't generate textual .dasm files.

src/coreclr/scripts/superpmi.py

src/coreclr/scripts/superpmi_asmdiffs_checked_release.py

JulieLeeMSFT · 2022-02-02T05:42:27Z

Hi @BruceForstall, it is ready for review.

The pipeline caught two asm diffs - Link.

unix-arm64
-- ISSUE: <ASM_DIFF> main method 246505 of size 2418 differs
win-arm64
-- ISSUE: <ASM_DIFF> main method 249554 of size 2418 differs

BruceForstall · 2022-02-04T06:20:28Z

I opened #64793 to track the asm diff.

BruceForstall

A few small suggestions

src/coreclr/scripts/superpmi.py

eng/pipelines/coreclr/superpmi-asmdiffs-checked-release.yml

src/coreclr/scripts/superpmi_asmdiffs_checked_release_setup.py

BruceForstall · 2022-02-16T03:43:57Z

Looks like there are new checked/release asmdiffs

JulieLeeMSFT · 2022-02-18T19:17:19Z

Looks like there are new checked/release asmdiffs

pipelink link
asm diff in coreclr_tests.pmi.Linux.arm64.checked.mch

win-arm64 log:
-- 323185e9-208a-4b35-a413-23f9aac2d5f7.windows.arm64\coreclr_tests.pmi.windows.arm64.checked.mch
-- ISSUE: <ASM_DIFF> main method 249555 of size 2418 differs
unix-arm64 log:
-- 323185e9-208a-4b35-a413-23f9aac2d5f7.Linux.arm64\coreclr_tests.pmi.Linux.arm64.checked.mch
-- ISSUE: <ASM_DIFF> main method 246450 of size 2418 differs

The detected method numbers are different from the last diff.

Created the first file that builds release and checked builds

* Added downloading JIT release builds in superpmi-asmdiff-job * Added dummy 'run-superpmi-asmdiff-job'

* Added JIT release download * Added dummy run job that just runs superpmi replay

* Remove the 4th argument for jobName and displayName

* In run-superpmi-asmdiff-checked-release-job.yml, * Added release_directory and release artifact to the script arguments * Added ReleaseCorrelationPayloadDirectory and ReleaseArtifactName to parameters to send-to-helix-step.yml * In superpmi_asmdiff_checked_release_setup.py * Added release_directory and release_artifactname arguments * Copied release binaries to source_directory/payload_release

* Added superpmi_asmdiffs_checked_release_setup.py to set up Helix directories for checked in base and release in diff directories. * Use superpmi_asmdiffs.py and superpmi-asmdiffs.proj. * Renamed files from asmdiff to asmdiffs.

…ile that compare with Release

…a boolean arg

…e build

…t removed from merge

… check with release

…ode 2) if --diff_with_release

…move jit-analyze and diff_summary.md

…ad of base_directory

…undays only.

JulieLeeMSFT · 2022-03-02T23:32:42Z

Failures are unrelated to this PR. Merging it.

JulieLeeMSFT · 2022-03-02T23:45:19Z

@echesakovMSFT, I see many REL32 error from today's run. Bruce said that you fixed it, and you might know about this error. Any idea?
LInk
ERROR: REL32 relocation overflows field! delta=0xFFFFFFFF6646CB48

ghost added the area-Infrastructure-coreclr label Nov 9, 2021

JulieLeeMSFT force-pushed the 49949 branch from 18af2a6 to 432fac8 Compare November 13, 2021 03:53

JulieLeeMSFT self-assigned this Nov 13, 2021

JulieLeeMSFT added this to the 7.0.0 milestone Nov 13, 2021

JulieLeeMSFT force-pushed the 49949 branch 2 times, most recently from 6eba786 to 2b3269f Compare December 2, 2021 20:06

ghost closed this Jan 1, 2022

JulieLeeMSFT reopened this Jan 26, 2022

JulieLeeMSFT force-pushed the 49949 branch from 2b3269f to ac129a3 Compare January 28, 2022 01:31

JulieLeeMSFT marked this pull request as ready for review January 28, 2022 01:34

JulieLeeMSFT requested a review from BruceForstall January 28, 2022 23:51

BruceForstall suggested changes Jan 29, 2022

View reviewed changes

ghost added needs-author-action An issue or pull request that requires more info or actions from the author. and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels Jan 29, 2022

BruceForstall mentioned this pull request Feb 4, 2022

JIT: checked/release asm diff in GitHub_17777 #64793

Closed

JulieLeeMSFT requested a review from BruceForstall February 4, 2022 18:40

BruceForstall suggested changes Feb 8, 2022

View reviewed changes

src/coreclr/scripts/superpmi.py Outdated Show resolved Hide resolved

eng/pipelines/coreclr/superpmi-asmdiffs-checked-release.yml Outdated Show resolved Hide resolved

src/coreclr/scripts/superpmi_asmdiffs_checked_release_setup.py Outdated Show resolved Hide resolved

ghost added the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 8, 2022

ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 16, 2022

JulieLeeMSFT added 2 commits March 1, 2022 12:14

SPMI no ASMDiff between Checked and Release

efc24d4

Created the first file that builds release and checked builds

SuperPMI asmdiff checked and release

3a78a69

* Added downloading JIT release builds in superpmi-asmdiff-job * Added dummy 'run-superpmi-asmdiff-job'

JulieLeeMSFT added 15 commits March 1, 2022 12:14

SuperPmi asmdiff checked release

1959d1d

* Added JIT release download * Added dummy run job that just runs superpmi replay

SuperPMI asmdiff checked release

a9bad61

* Remove the 4th argument for jobName and displayName

SuperPMI asmdiffs checked release

5c6dddf

* Added superpmi_asmdiffs_checked_release_setup.py to set up Helix directories for checked in base and release in diff directories. * Use superpmi_asmdiffs.py and superpmi-asmdiffs.proj. * Renamed files from asmdiff to asmdiffs.

SuperPMI asmdiffs checked release: resolve merge conflict

16fdb6e

SuperPMI asmdiffs checked release: Fixed to use a proj and a script f…

3f80738

…ile that compare with Release

Superpmi asmdiffs checked release: changed to --diff_with_release as …

84a456e

…a boolean arg

SuperPMI asmdiffs checked release: Add dependency to completed Releas…

30e8de1

…e build

Superpmi asmdiffs checked release: add diff_with_release back that go…

75b1d60

…t removed from merge

SuperPMI asmdiffs checked release: Do not create asm file for asmdiff…

1fc1380

… check with release

SuperPMI asmdiffs checked release: Report failure asm diffs (return c…

21af7b7

…ode 2) if --diff_with_release

SuperPMI asmdiffs checked release: Handled code review feedback to re…

77328b6

…move jit-analyze and diff_summary.md

SuperPMI asmdiffs checked release: Changed to checked_directory inste…

cf3ecf7

…ad of base_directory

SuperPMI asmdiffs checked release: explictly say checked_directory

fd9ad6c

SuperPMI asmdiffs checked release: schedule to run on Saturdays and S…

8537008

…undays only.

JulieLeeMSFT force-pushed the 49949 branch from d4a851d to 8537008 Compare March 2, 2022 00:28

BruceForstall approved these changes Mar 2, 2022

View reviewed changes

JulieLeeMSFT merged commit 7830fdd into dotnet:main Mar 2, 2022

JulieLeeMSFT deleted the 49949 branch March 2, 2022 23:33

This was referenced Mar 2, 2022

Create checked/release SuperPMI asm diffs AzDO pipeline #49949

Closed

Automate Diffing of Release, Checked and Debug JITs #5884

Closed

JulieLeeMSFT mentioned this pull request Mar 3, 2022

JIT: checked/release asm diff in windows and linux x64 #66155

Closed

ghost locked as resolved and limited conversation to collaborators Apr 2, 2022

Conversation

JulieLeeMSFT commented Nov 9, 2021

Uh oh!

ghost commented Nov 9, 2021

Uh oh!

JulieLeeMSFT commented Nov 13, 2021

Uh oh!

ghost commented Jan 1, 2022

Uh oh!

JulieLeeMSFT commented Jan 28, 2022

Uh oh!

BruceForstall commented Jan 29, 2022

Uh oh!

BruceForstall left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JulieLeeMSFT commented Feb 2, 2022

Uh oh!

BruceForstall commented Feb 4, 2022

Uh oh!

BruceForstall left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BruceForstall commented Feb 16, 2022

Uh oh!

JulieLeeMSFT commented Feb 18, 2022

Uh oh!

JulieLeeMSFT commented Mar 2, 2022

Uh oh!

JulieLeeMSFT commented Mar 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants