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.

Sign up for GitHub

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

Jump to bottom

Remove old multi-dimensional array code #71678

Closed
Tracked by #77032
BruceForstall opened this issue Jul 5, 2022 · 2 comments · Fixed by #86133
Closed
Tracked by #77032

Remove old multi-dimensional array code #71678

BruceForstall opened this issue Jul 5, 2022 · 2 comments · Fixed by #86133
Assignees
@BruceForstall
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone
8.0.0

Comments

@BruceForstall
Copy link
Member

BruceForstall commented Jul 5, 2022 •
edited
Loading

After #70271, the GT_ARR_ELEM, GT_ARR_OFFSET, and GT_ARR_INDEX nodes, and their respective GenTree structures and all related codegen, are unneeded and are dead code.

Once we are confident in the direction of #70271 (confident that we don't want to revert to the old mechanism), we should remove all this dead code, as well as the JitEarlyExpandMDArrays and JitEarlyExpandMDArraysFilter configuration values.

category:cq
theme:md-arrays
skill-level:beginner
cost:small
impact:small
@BruceForstall BruceForstall added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 5, 2022
@BruceForstall BruceForstall added this to the 8.0.0 milestone Jul 5, 2022
@BruceForstall BruceForstall self-assigned this Jul 5, 2022
@ghost
Copy link

ghost commented Jul 5, 2022

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

Issue Details

After #70271, the GT_ARR_ELEM, GT_ARR_OFFSET, and GT_ARR_INDEX nodes, and their respective GenTree structures and all related codegen, are unneeded and are dead code.

Once we are confident in the direction of #70271 (confident that we don't want to revert to the old mechanism), we should remove all this dead code, as well as the JitEarlyExpandMDArrays and JitEarlyExpandMDArraysFilter configuration values.

Author: BruceForstall
Assignees: BruceForstall
Labels:

area-CodeGen-coreclr
Milestone: 8.0.0

@BruceForstall BruceForstall mentioned this issue Jul 5, 2022
Closed
@BruceForstall
Copy link
Member Author

Somewhat related:

There is a StyleCop rule recommending avoiding multi-dimensional arrays:

CA1814: Prefer jagged arrays over multidimensional (code analysis) - .NET | Microsoft Docs

Interestingly, the documentation emphasizes that this is for size, not performance, reasons. Seems extremely unlikely the size case is what led to the rule or that size concerns are frequent when making a choice between the two types. Also seems like a post-hoc explanation, given that the documentation text was relatively recently changed to emphasize this: Update CA1814 and jagged arrays by gewarren · Pull Request #22088 · dotnet/docs (github.com)

It’s not wrong, but possibly misleading if someone sees that warning but doesn’t read the explanation.

Consider getting this rule removed.

This was referenced Oct 13, 2022
Closed
Open
BruceForstall added a commit to BruceForstall/runtime that referenced this issue May 12, 2023
@BruceForstall
Remove unused multi-dimensional array code
62ac571 
MD array processing in the JIT changed with dotnet#70271,
making all the code that implemented the old processing method unused.

Remove all this unused code.

Fixes dotnet#71678
@BruceForstall BruceForstall mentioned this issue May 12, 2023
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 12, 2023
BruceForstall added a commit that referenced this issue May 12, 2023
@BruceForstall
Remove unused multi-dimensional array code (#86133)
93a811f 
MD array processing in the JIT changed with #70271,
making all the code that implemented the old processing method unused.

Remove all this unused code.

Fixes #71678
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 12, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@BruceForstall BruceForstall

Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Milestone
8.0.0
Development

Successfully merging a pull request may close this issue.

Remove unused multi-dimensional array code BruceForstall/runtime
1 participant
@BruceForstall