Fix bug in String.Equals unrolling for certain single-char strings #78190
Conversation
ghost
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsFixes #78169 bug For single-char strings with "signed" bit (e.g.
|
|
/azp run runtime-coreclr outerloop, runtime-coreclr jitstress, runtime-coreclr gcstress0x3-gcstress0xc |
|
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Coding guidelines I think say to always use ASCII, ie use \uNNNN to avoid accidental garbling later. I don't know whether we are actually consistent. But in this case it would perhaps be clearer.
There was a problem hiding this comment.
Thanks, changed to \u9244
There was a problem hiding this comment.
There are some existing non-ASCII chars in this file:
https://gist.github.com/am11/dddd538ad08af8c914ba1144adf74263#file-stringequals-cs
There was a problem hiding this comment.
Thanks! Let's keep this PR smaller for easier/cleaner backport. @am11 feel free to file a PR to clean that up after
There was a problem hiding this comment.
feel free to file a PR to clean that up after
|
@dotnet/jit-contrib @jakobbotsch PTAL, I'm going to backport it once merged |
EgorBo
force-pushed
the
fix-equals-unroll-bug
branch
from
33f4849 to
4e476cf
Compare
Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
|
/backport to release/7.0 |
|
Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3446166446 |
|
@EgorBo backporting to release/7.0 failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Use cast nodes
Using index info to reconstruct a base tree...
A src/coreclr/jit/importervectorization.cpp
Falling back to patching base and 3-way merge...
Auto-merging src/coreclr/jit/importer_vectorization.cpp
CONFLICT (content): Merge conflict in src/coreclr/jit/importer_vectorization.cpp
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Use cast nodes
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
|
@EgorBo an error occurred while backporting to release/7.0, please check the run log for details! Error: git am failed, most likely due to a merge conflict. |
|
SPMI failures are due to today's JIT-EE guid update |
6 participants
Fixes #78169 bug
For single-char strings with "signed" bit (e.g.
((ushort)'ι') >> 15) we used to do sign-extension instead of zero-extension πFrom my understanding, only single-char strings (length == 1) of certain two-bytes languages/symbols are affected (looks like mostly around CJK languages). Still, definitely needs to be backported to 7.0