Skip to content

Conversation

davidwrighton
Copy link
Member

  • use static_asserts where possible
  • remove the mess of casts and temp variables around helper function opcodes
  • Remove the DO_GENERIC_LOOKUP macro since it wasn't doing much of use anymore
  • Use the simdhash apis a little better

- use static_asserts where possible
- remove the mess of casts and temp variables around helper function opcodes
- Remove the DO_GENERIC_LOOKUP macro since it wasn't doing much of use anymore
- Use the simdhash apis a little better
@Copilot Copilot AI review requested due to automatic review settings June 27, 2025 17:35
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jun 27, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR makes several follow-on improvements to the interpreter-related code by reducing cast and temporary variable usage, replacing the DO_GENERIC_LOOKUP macro with inline calls to DoGenericLookup, and improving the use of simdhash APIs while enhancing compile‐time assertions.

  • Remove unnecessary casts and temp variables in helper function calls
  • Replace DO_GENERIC_LOOKUP macro with explicit calls to DoGenericLookup
  • Update static assertions to use static_assert_no_msg and minor brace/indentation cleanups

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/native/containers/dn-simdhash-specialization-declarations.h Added comments clarifying that the result parameter is optional
src/coreclr/vm/interpexec.cpp Updated helper typedef names, replaced macro usage with inline DoGenericLookup calls, and adjusted helper call invocations
src/coreclr/interpreter/compiler.h Replaced runtime asserts with static_assert_no_msg and modified braces to improve code clarity
Comments suppressed due to low confidence (1)

src/coreclr/vm/interpexec.cpp:1771

  • Ensure that the revised generic lookup logic in InterpExecMethod—especially the inline DoGenericLookup invocations—is thoroughly exercised by unit tests to cover all branch variations.
                    MethodTable *pMTNewObj = (MethodTable*)DoGenericLookup(LOCAL_VAR(ip[3], void*), pLookup);

@davidwrighton davidwrighton merged commit 2770734 into dotnet:main Jun 27, 2025
150 of 156 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants