Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Undo a few JIT layout workarounds - #23510

Merged
jkotas merged 1 commit into
dotnet:masterfrom
JosephTremoulet:UndoGoto
Aug 24, 2017
Merged

jkotas merged 1 commit into
dotnet:masterfrom
JosephTremoulet:UndoGoto

Conversation

@JosephTremoulet

Copy link
Copy Markdown
Contributor

Remove some gotos that were added to work around dotnet/coreclr#9692
(poor code layout for loop exit paths) -- the JIT's layout decisions
were improved in dotnet/coreclr#13314, and these particular gotos are
no longer needed; the same machine code is generated with or without
this change.
Some gotos previously tagged as workarounds for dotnet/coreclr#9692 are
still relevant for keeping codesize down pending dotnet/coreclr#13549;
update their comments accordingly.

Part of #23395.

@@ -456,30 +428,22 @@ public static unsafe int IndexOfAny(ref byte searchSpace, byte value0, byte valu
}
// Found match, reuse Vector vComparison to keep register pressure low

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.

You should be able to remove this bit too and just use vMatches to compute the result.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated (x3)

@@ -303,30 +287,22 @@ public static unsafe int IndexOfAny(ref byte searchSpace, byte value0, byte valu
}
// Found match, reuse Vector vComparison to keep register pressure low
values0 = vMatches;

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.

Same here

Remove some `goto`s that were added to work around dotnet/coreclr#9692
(poor code layout for loop exit paths) -- the JIT's layout decisions
were improved in dotnet/coreclr#13314, and these particular `goto`s are
no longer needed; the same machine code is generated with or without
this change.
Some `goto`s previously tagged as workarounds for dotnet/coreclr#9692 are
still relevant for keeping codesize down pending dotnet/coreclr#13549;
update their comments accordingly.

Part of #23395.
index += Vector<byte>.Count;
continue;
}
// Found match, reuse Vector vComparison to keep register pressure low

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this workaround is also not necessary?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's correct

@jkotas

jkotas commented Aug 24, 2017

Copy link
Copy Markdown
Member

@dotnet-bot test Windows x64 Debug Build please
@dotnet-bot test Linux x64 Release Build please

(System.Private.Uri.Functional crash)

@jkotas
jkotas merged commit d90ebb8 into dotnet:master Aug 24, 2017
@JosephTremoulet
JosephTremoulet deleted the UndoGoto branch August 28, 2017 15:18
@karelz karelz modified the milestone: 2.1.0 Aug 28, 2017
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Remove some `goto`s that were added to work around dotnet/coreclrdotnet/corefx#9692
(poor code layout for loop exit paths) -- the JIT's layout decisions
were improved in dotnet/coreclrdotnet/corefx#13314, and these particular `goto`s are
no longer needed; the same machine code is generated with or without
this change.
Some `goto`s previously tagged as workarounds for dotnet/coreclrdotnet/corefx#9692 are
still relevant for keeping codesize down pending dotnet/coreclrdotnet/corefx#13549;
update their comments accordingly.

Part of dotnet/corefx#23395.

Commit migrated from dotnet/corefx@d90ebb8
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.

6 participants