Skip to content

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