Skip to content

Delete GT_SIMD_CHK and GT_HW_INTRINSIC_CHK - #62088

Merged
echesakov merged 4 commits into
dotnet:mainfrom
SingleAccretion:More-Range-Check-Simplification
Dec 16, 2021
Merged

Delete GT_SIMD_CHK and GT_HW_INTRINSIC_CHK#62088
echesakov merged 4 commits into
dotnet:mainfrom
SingleAccretion:More-Range-Check-Simplification

Conversation

@SingleAccretion

@SingleAccretionSingleAccretion commented Nov 26, 2021

Copy link
Copy Markdown
Contributor

The former was unused, the latter - barely used. Overall, there is no need for these to be separate opers, it just leads to unnecessary #ifdefs.

There are two commits with this change: first just mechanically deletes the opers, leaving for back-compat places where the array checks were treated differently. The second commit removes the distinction as it is not a useful one.

Since the bounds checks are (no longer) array-only, the remaining oper was renamed to simply GT_BOUNDS_CHECK.

Some diffs from the second commit, in particular the assertion propagation change.

@ghostghost added community-contribution Indicates that the PR has been added by a community member area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Nov 26, 2021
@ghost

Copy link
Copy Markdown

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

Issue Details

The former was unused, the latter - barely used. Overall, there is no need for these to be separate opers, it just leads to unnecessary #ifdefs.

There are two commits with this change: first just mechanically the opers, leaving for back-compat places where the array checks were treated different as they were. The second commit removes the distinction as it is not a useful one.

Since the bounds checks are (no longer) array-only, the remaining oper was renamed to simply GT_BOUNDS_CHECK.

Author:SingleAccretion
Assignees:-
Labels:

area-CodeGen-coreclr, community-contribution

Milestone:-

@SingleAccretion
SingleAccretionforce-pushed the More-Range-Check-Simplification branch from 366dd06 to 01de691CompareNovember 26, 2021 22:20
@SingleAccretion
SingleAccretion marked this pull request as ready for review November 28, 2021 14:23
@SingleAccretion

Copy link
Copy Markdown
ContributorAuthor

@dotnet/jit-contrib

jakobbotsch
jakobbotsch previously approved these changes Nov 29, 2021
@jakobbotsch

Copy link
Copy Markdown
Member

Whoops, that approval was meant for #61911. 😄

@SingleAccretion
SingleAccretionforce-pushed the More-Range-Check-Simplification branch from 5ac62d2 to 722f8e0CompareDecember 1, 2021 12:13
@echesakov
echesakov self-requested a review December 3, 2021 03:58
The former was unused, the latter - barely used.
Overall, there is no need for these to be separate
opers, it just leads to unnecessary `#ifdef`s.
In loop cloning: redundant with the check for constant lengths below.
In assertion propagation: needless pessimization.
In range check: not needed. While it is not useful to search for the
array lengths from "new int[] { ... }" expressions, it is still useful
to look for assertions related to them.
@SingleAccretion
SingleAccretionforce-pushed the More-Range-Check-Simplification branch from 722f8e0 to 866b487CompareDecember 10, 2021 17:59

@AndyAyersMSAndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@AndyAyersMS

Copy link
Copy Markdown
Member

@echesakov are you going to review as well?

@echesakov
echesakov merged commit e7de86f into dotnet:mainDec 16, 2021
@SingleAccretion
SingleAccretion deleted the More-Range-Check-Simplification branch December 17, 2021 09:28
@ghostghost locked as resolved and limited conversation to collaborators Jan 16, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMIcommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@SingleAccretion@jakobbotsch@AndyAyersMS@echesakov