Skip to content

Commit

Permalink
Upload dasm files as artifacts for "asmdiffs pipeline" (#61700)
Browse files Browse the repository at this point in the history
* Temp change to disable align loop

* download specific artifacts

to squash:

* Upload .dasm files

* fix the build id

* Add ci_run and retainOnlyTopFiles

* Rename ci_run to retainOnlyTop

* Disable struct promo to test asmdiff

* Revert "Disable struct promo to test asmdiff"

This reverts commit 3ef7adb.

* fix the parameter retainOnlyTopFiles

* add missing -

* Revert "Temp change to disable align loop"

This reverts commit b1de5c4.

* Revert "download specific artifacts"

This reverts commit db3de57.

* Review comments
  • Loading branch information
kunalspathak authored Nov 18, 2021
1 parent 30550d6 commit 66b31ca
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 7 deletions.
8 changes: 8 additions & 0 deletions eng/pipelines/coreclr/templates/run-superpmi-asmdiffs-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,14 @@ jobs:
targetFolder: '$(SpmiAsmdiffsLocation)'
condition: always()

- task: CopyFiles@2
displayName: Copying dasm files of all partitions
inputs:
sourceFolder: '$(HelixResultLocation)'
contents: '**/Asmdiffs_*.zip'
targetFolder: '$(SpmiAsmdiffsLocation)'
condition: always()

- script: $(PythonScript) $(Build.SourcesDirectory)/src/coreclr/scripts/superpmi_asmdiffs_summarize.py -diff_summary_dir $(SpmiAsmdiffsLocation) -arch $(archType)
displayName: ${{ format('Summarize ({0})', parameters.archType) }}
condition: always()
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/scripts/superpmi-asmdiffs.proj
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
<HelixWorkItem Include="@(SPMI_Partition)">
<Command>$(WorkItemCommand) -arch %(HelixWorkItem.Architecture) -platform %(HelixWorkItem.Platform)</Command>
<Timeout>$(WorkItemTimeout)</Timeout>
<DownloadFilesFromResults>superpmi_%(HelixWorkItem.Platform)_%(HelixWorkItem.Architecture).log;superpmi_download_%(HelixWorkItem.Platform)_%(HelixWorkItem.Architecture).log;superpmi_diff_summary_%(HelixWorkItem.Platform)_%(HelixWorkItem.Architecture).md</DownloadFilesFromResults>
<DownloadFilesFromResults>superpmi_%(HelixWorkItem.Platform)_%(HelixWorkItem.Architecture).log;superpmi_download_%(HelixWorkItem.Platform)_%(HelixWorkItem.Architecture).log;superpmi_diff_summary_%(HelixWorkItem.Platform)_%(HelixWorkItem.Architecture).md;Asmdiffs_%(HelixWorkItem.Platform)_%(HelixWorkItem.Architecture).zip</DownloadFilesFromResults>
</HelixWorkItem>
</ItemGroup>
</Project>
8 changes: 8 additions & 0 deletions src/coreclr/scripts/superpmi.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@
asm_diff_parser.add_argument("-diff_jit_option", action="append", help="Option to pass to the diff JIT. Format is key=value, where key is the option name without leading COMPlus_...")
asm_diff_parser.add_argument("-tag", help="Specify a word to add to the directory name where the asm diffs will be placed")
asm_diff_parser.add_argument("-metrics", action="append", help="Metrics option to pass to jit-analyze. Can be specified multiple times, or pass comma-separated values.")
asm_diff_parser.add_argument("-retainOnlyTopFiles", action="store_true", help="Retain only top .dasm files with largest improvements or regressions and delete remaining files.")

# subparser for upload
upload_parser = subparsers.add_parser("upload", description=upload_description, parents=[core_root_parser, target_parser])
Expand Down Expand Up @@ -1628,6 +1629,8 @@ async def create_one_artifact(jit_path: str, location: str, flags) -> str:
summary_file_info = ( mch_file, md_summary_file )
all_md_summary_files.append(summary_file_info)
command = [ jit_analyze_path, "--md", md_summary_file, "-r", "--base", base_asm_location, "--diff", diff_asm_location ]
if self.coreclr_args.retainOnlyTopFiles:
command += [ "--retainOnlyTopFiles" ]
if self.coreclr_args.metrics:
command += [ "--metrics", ",".join(self.coreclr_args.metrics) ]
elif base_bytes is not None and diff_bytes is not None:
Expand Down Expand Up @@ -3261,6 +3264,11 @@ def verify_replay_common_args():
lambda unused: True,
"Unable to set metrics.")

coreclr_args.verify(args,
"retainOnlyTopFiles",
lambda unused: True,
"Unable to set retainOnlyTopFiles.")

process_base_jit_path_arg(coreclr_args)

jit_in_product_location = False
Expand Down
60 changes: 54 additions & 6 deletions src/coreclr/scripts/superpmi_asmdiffs.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@
################################################################################

import argparse
import os
from os import walk, path
import shutil
from coreclr_arguments import *
from jitutil import run_command
from jitutil import run_command, TempDir

parser = argparse.ArgumentParser(description="description")

Expand Down Expand Up @@ -66,6 +66,54 @@ def setup_args(args):

return coreclr_args

def copy_dasm_files(spmi_location, upload_directory, tag_name):
"""Copies .dasm files to a tempDirectory, zip it, and copy the compressed file to the upload directory.
Args:
spmi_location (string): Location where .dasm files are present
upload_directory (string): Upload directory
tag_name (string): tag_name used in zip file name.
"""

print("Copy .dasm files")

# Create upload_directory
if not os.path.isdir(upload_directory):
os.makedirs(upload_directory)

# Create temp directory to copy all issues to upload. We don't want to create a sub-folder
# inside issues_directory because that will also get included twice.
with TempDir() as prep_directory:
for file_path, dirs, files in walk(spmi_location, topdown=True):
# Credit: https://stackoverflow.com/a/19859907
dirs[:] = [d for d in dirs]
for name in files:
if not name.lower().endswith(".dasm"):
continue

dasm_src_file = path.join(file_path, name)
dasm_dst_file = dasm_src_file.replace(spmi_location, prep_directory)
dst_directory = path.dirname(dasm_dst_file)
if not os.path.exists(dst_directory):
os.makedirs(dst_directory)
try:
shutil.copy2(dasm_src_file, dasm_dst_file)
except PermissionError as pe_error:
print('Ignoring PermissionError: {0}'.format(pe_error))

# Zip compress the files we will upload
zip_path = os.path.join(prep_directory, "Asmdiffs_" + tag_name)
print("Creating archive: " + zip_path)
shutil.make_archive(zip_path, 'zip', prep_directory)

zip_path += ".zip"
dst_zip_path = os.path.join(upload_directory, "Asmdiffs_" + tag_name + ".zip")
print("Copying {} to {}".format(zip_path, dst_zip_path))
try:
shutil.copy2(zip_path, dst_zip_path)
except PermissionError as pe_error:
print('Ignoring PermissionError: {0}'.format(pe_error))


def main(main_args):
""" Run superpmi asmdiffs process on the Helix machines.
Expand Down Expand Up @@ -156,7 +204,8 @@ def main(main_args):
"-spmi_location", spmi_location,
"-error_limit", "100",
"-log_level", "debug",
"-log_file", log_file])
"-log_file", log_file,
"-retainOnlyTopFiles"])

# If there are asm diffs, and jit-analyze ran, we'll get a diff_summary.md file in the spmi_location directory.
# We make sure the file doesn't exist before we run diffs, so we don't need to worry about superpmi.py creating
Expand All @@ -169,6 +218,8 @@ def main(main_args):
shutil.copy2(overall_md_summary_file, overall_md_summary_file_target)
except PermissionError as pe_error:
print('Ignoring PermissionError: {0}'.format(pe_error))

copy_dasm_files(spmi_location, log_directory, "{}_{}".format(platform_name, arch_name))
else:
# Write a basic summary file. Ideally, we should not generate a summary.md file. However, currently I'm seeing
# errors where the Helix work item fails to upload this specified file if it doesn't exist. We should change the
Expand All @@ -178,9 +229,6 @@ def main(main_args):
No diffs found
""")

# TODO: the superpmi.py asmdiffs command returns a failure code if there are MISSING data even if there are
# no asm diffs. We should probably only fail if there are actual failures (not MISSING or asm diffs).

if return_code != 0:
print("Failure in {}".format(log_file))
return 1
Expand Down

0 comments on commit 66b31ca

Please sign in to comment.