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

Port #39096 - Implementation of CSE for large constants on ARM64 #39883

Merged
merged 1 commit into from
Jul 24, 2020

Conversation

briansull
Copy link
Contributor

  • Implementation of code size optimization, CSE of large constants for ARM64

Implementation of code size optimization, CSE of large constant for ARM64
We will share a single CSE for constants that differ only in their low 12 bits on ARM64

Config variable: COMPlus_JitConstCSE
// Default 0: enable the CSE of Constants, including nearby offsets. (only for ARM64)
// If 1, disable all the CSE of Constants
// If 2, enable the CSE of Constants but don't combine with nearby offsets. (only for ARM64)
// If 3, enable the CSE of Constants including nearby offsets. (all platforms)
// If 4, enable the CSE of Constants but don't combine with nearby offsets. (all platforms)
//

  • Change the type of csdHashKey to size_t
  • Update gtCostSz and gtCostEx for constant nodes
  • Added additional Priority 0 test coverage for Floating Point optimizations

* Change the type of csdHashKey to size_t

* Update gtCostSz and gtCostEx for constant nodes

* Implementation of code size optimization, CSE of constant values for ARM64
Implementation of code size optimization, CSE of constant values for ARM64
We will share a single CSE for constants that differ only in their low 12 bits on ARM64

Number of shared constant low bits set in target.h  CSE_CONST_SHARED_LOW_BITS
we use 12 bits on Arm platforms and 16 bits on XArch platforms

Disable the CSE of the REG_R2R_INDIRECT_PARAM on Arm32
as it hits  Assertion failed 'candidates != candidateBit' in lsra.cpp Line: 3723

Config variable: COMPlus_JitConstCSE
// Default 0: enable the CSE of Constants, including nearby offsets. (only for ARM64)
// If 1, disable all the CSE of Constants
// If 2, enable the CSE of Constants but don't combine with nearby offsets. (only for ARM64)
// If 3, enable the CSE of Constants including nearby offsets. (all platforms)
// If 4, enable the CSE of Constants but don't combine with nearby offsets. (all platforms)
//

* Added additional Priority 0 test coverage for Floating Point optimizations

* Fix for COMPLUS_JitConstCSE=4

* Renamed config variable from COMPlus_JitDisableConstCSE to COMPlus_JitConstCSE

* Updated with Codereview feedback, removed sort from Const CSE phase

* Fix for assertionProp issue in the refTypesdynamic test
@briansull
Copy link
Contributor Author

briansull commented Jul 24, 2020

I believe that the test failure is due to: #39473

runtime (Build Browser wasm Release AllSubsets_Mono) Failing after 74m — Build Browser wasm Release AllSubsets_Mono failed

This is what I found in the console log for the test leg:
https://helix.dot.net/api/2019-06-17/jobs/8ee35f01-bc0c-48a1-bd50-7a4e28e0dfa5/workitems/System.Formats.Asn1.Tests/console

[03:36:03] dbug: test[0]
      [FAIL] System.Formats.Asn1.Tests.Writer.WriteOctetString.WriteSegmentedCER
      System.NullReferenceException : Object reference not set to an instance of an object.
         at System.Reflection.CustomAttributeTypedArgument.CanonicalizeValue(Object value)
         at System.Reflection.CustomAttributeTypedArgument..ctor(Type argumentType, Object value)
         at System.Reflection.CustomAttributeData.ResolveArguments()
         at System.Reflection.CustomAttributeData.get_ConstructorArguments()
         at ReflectionAbstractionExtensions.GetCustomAttributes(IMethodInfo methodInfo, Type attributeType)

@briansull
Copy link
Contributor Author

@jeffschwMSFT PTAL

@briansull
Copy link
Contributor Author

@briansull briansull merged commit d999d04 into dotnet:release/5.0-preview8 Jul 24, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants