[mini] Fix typo in mono_decompose_vtype_opts - #90825

Merged
lewing merged 2 commits into
dotnet:mainfrom
lambdageek:fix-gh-90800
Aug 19, 2023
Merged

[mini] Fix typo in mono_decompose_vtype_opts#90825
lewing merged 2 commits into
dotnet:mainfrom
lambdageek:fix-gh-90800

Conversation

@lambdageek

@lambdageeklambdageek commented Aug 18, 2023

Copy link
Copy Markdown
Member

Without this, if some previous instruction already created a vreg for ins->dest (for example if we are doing multiple passes over the basic block because restart == TRUE) we will use an incorrect vreg for the src when decomposing the current VMOVE


Additionally, only emit OP_LDTOKEN_FIELD if we loaded a field token

This is used by a CreateSpan intrinsic #81695 that needs access to the MonoClassField*

For other cases of a bare LDTOKEN (such as hand-written IL that calls LDTOKEN on a type but doesn't follow it up with a call to GetTypeFromHandle leave the opcode as a VMOVE (from the EMIT_NEW_TEMPLOAD above))

Fixes#90800

Without this, if some previous instruction already created a vreg for
ins->dest (for example if we are doing multiple passes over the basic
block because `restart == TRUE`) we will use an incorrect vreg when
decomposing the current VMOVE
Fixesdotnet#90800
This is used by a CreateSpan optimization that needs access to the
MonoClassField*
For other cases of a bare LDTOKEN (such as hand-written IL that calls
LDTOKEN on a type but doesn't follow it up with a call to
`GetTypeFromHandle` leave the opcode as a VMOVE (from the
`EMIT_NEW_TEMPLOAD` above))
@lambdageek

Copy link
Copy Markdown
MemberAuthor

TODO: need to add a regression test.

attn @SamMonoRT we will need to backport this to net8.

@lambdageek

Copy link
Copy Markdown
MemberAuthor

@simonrozsival@rolfbjarne do we need to try to get this into rc1, is it completely blocking ios?

As a workaround, emitting roslyn-style IL will avoid this issue (because there will be an intervening callvirt that will avoid the decompose bug)
https://sharplab.io/#v2:C4LglgNgPgAgTARgLACgDEA7ArhCBDAIwgFMACAEzAGdCTVSHTUYBmU+UgYVIG97HWpAEpYMwMAFtiAFQCeAB2IAJPBnIlSAWQAUASl79GAgOylgC4gHsAZtpgIADLoB0cxSrUkA3IdIBfVD8gA=

ie instead of ldtoken System.Runtime]System.String emit

 IL_0000: ldtoken [System.Runtime]System.String
IL_0005: call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle)
IL_000a: callvirt instance valuetype [System.Runtime]System.RuntimeTypeHandle [System.Runtime]System.Type::get_TypeHandle()

@lambdageeklambdageek added this to the 8.0.0 milestone Aug 18, 2023
@lewing

Copy link
Copy Markdown
Member

if this is a blocker for rc1 we should open the backport now and send the email

@lambdageek

Copy link
Copy Markdown
MemberAuthor

/backport to release/8.0-rc1

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/8.0-rc1: https://github.com/dotnet/runtime/actions/runs/5907349234

@lambdageek

lambdageek commented Aug 18, 2023

Copy link
Copy Markdown
MemberAuthor

@lewing I'm offline for at least another 3 hours. Can you send the email - see the comments in the issue for details I sent the email. Waiting on a decision...

@lewing
lewing merged commit 940b332 into dotnet:mainAug 19, 2023
@ghostghost locked as resolved and limited conversation to collaborators Sep 18, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Mono] RuntimeTypeHandle.Equals seems to be broken in Release mode

3 participants

@lambdageek@lewing@SamMonoRT
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

[mini] Fix typo in mono_decompose_vtype_opts - #90825

Merged
lewing merged 2 commits into
dotnet:mainfrom
lambdageek:fix-gh-90800
Aug 19, 2023
Merged

[mini] Fix typo in mono_decompose_vtype_opts#90825
lewing merged 2 commits into
dotnet:mainfrom
lambdageek:fix-gh-90800

Conversation

@lambdageek

@lambdageeklambdageek commented Aug 18, 2023

Copy link
Copy Markdown
Member

Without this, if some previous instruction already created a vreg for ins->dest (for example if we are doing multiple passes over the basic block because restart == TRUE) we will use an incorrect vreg for the src when decomposing the current VMOVE


Additionally, only emit OP_LDTOKEN_FIELD if we loaded a field token

This is used by a CreateSpan intrinsic #81695 that needs access to the MonoClassField*

For other cases of a bare LDTOKEN (such as hand-written IL that calls LDTOKEN on a type but doesn't follow it up with a call to GetTypeFromHandle leave the opcode as a VMOVE (from the EMIT_NEW_TEMPLOAD above))

Fixes#90800

Without this, if some previous instruction already created a vreg for
ins->dest (for example if we are doing multiple passes over the basic
block because `restart == TRUE`) we will use an incorrect vreg when
decomposing the current VMOVE
Fixesdotnet#90800
This is used by a CreateSpan optimization that needs access to the
MonoClassField*
For other cases of a bare LDTOKEN (such as hand-written IL that calls
LDTOKEN on a type but doesn't follow it up with a call to
`GetTypeFromHandle` leave the opcode as a VMOVE (from the
`EMIT_NEW_TEMPLOAD` above))
@lambdageek

Copy link
Copy Markdown
MemberAuthor

TODO: need to add a regression test.

attn @SamMonoRT we will need to backport this to net8.

@lambdageek

Copy link
Copy Markdown
MemberAuthor

@simonrozsival@rolfbjarne do we need to try to get this into rc1, is it completely blocking ios?

As a workaround, emitting roslyn-style IL will avoid this issue (because there will be an intervening callvirt that will avoid the decompose bug)
https://sharplab.io/#v2:C4LglgNgPgAgTARgLACgDEA7ArhCBDAIwgFMACAEzAGdCTVSHTUYBmU+UgYVIG97HWpAEpYMwMAFtiAFQCeAB2IAJPBnIlSAWQAUASl79GAgOylgC4gHsAZtpgIADLoB0cxSrUkA3IdIBfVD8gA=

ie instead of ldtoken System.Runtime]System.String emit

 IL_0000: ldtoken [System.Runtime]System.String
IL_0005: call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle)
IL_000a: callvirt instance valuetype [System.Runtime]System.RuntimeTypeHandle [System.Runtime]System.Type::get_TypeHandle()

@lambdageeklambdageek added this to the 8.0.0 milestone Aug 18, 2023
@lewing

Copy link
Copy Markdown
Member

if this is a blocker for rc1 we should open the backport now and send the email

@lambdageek

Copy link
Copy Markdown
MemberAuthor

/backport to release/8.0-rc1

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/8.0-rc1: https://github.com/dotnet/runtime/actions/runs/5907349234

@lambdageek

lambdageek commented Aug 18, 2023

Copy link
Copy Markdown
MemberAuthor

@lewing I'm offline for at least another 3 hours. Can you send the email - see the comments in the issue for details I sent the email. Waiting on a decision...

@lewing
lewing merged commit 940b332 into dotnet:mainAug 19, 2023
@ghostghost locked as resolved and limited conversation to collaborators Sep 18, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Mono] RuntimeTypeHandle.Equals seems to be broken in Release mode

3 participants

@lambdageek@lewing@SamMonoRT
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

[mini] Fix typo in mono_decompose_vtype_opts - #90825

Merged
lewing merged 2 commits into
dotnet:mainfrom
lambdageek:fix-gh-90800
Aug 19, 2023
Merged

[mini] Fix typo in mono_decompose_vtype_opts#90825
lewing merged 2 commits into
dotnet:mainfrom
lambdageek:fix-gh-90800

Conversation

@lambdageek

@lambdageeklambdageek commented Aug 18, 2023

Copy link
Copy Markdown
Member

Without this, if some previous instruction already created a vreg for ins->dest (for example if we are doing multiple passes over the basic block because restart == TRUE) we will use an incorrect vreg for the src when decomposing the current VMOVE


Additionally, only emit OP_LDTOKEN_FIELD if we loaded a field token

This is used by a CreateSpan intrinsic #81695 that needs access to the MonoClassField*

For other cases of a bare LDTOKEN (such as hand-written IL that calls LDTOKEN on a type but doesn't follow it up with a call to GetTypeFromHandle leave the opcode as a VMOVE (from the EMIT_NEW_TEMPLOAD above))

Fixes#90800

Without this, if some previous instruction already created a vreg for
ins->dest (for example if we are doing multiple passes over the basic
block because `restart == TRUE`) we will use an incorrect vreg when
decomposing the current VMOVE
Fixesdotnet#90800
This is used by a CreateSpan optimization that needs access to the
MonoClassField*
For other cases of a bare LDTOKEN (such as hand-written IL that calls
LDTOKEN on a type but doesn't follow it up with a call to
`GetTypeFromHandle` leave the opcode as a VMOVE (from the
`EMIT_NEW_TEMPLOAD` above))
@lambdageek

Copy link
Copy Markdown
MemberAuthor

TODO: need to add a regression test.

attn @SamMonoRT we will need to backport this to net8.

@lambdageek

Copy link
Copy Markdown
MemberAuthor

@simonrozsival@rolfbjarne do we need to try to get this into rc1, is it completely blocking ios?

As a workaround, emitting roslyn-style IL will avoid this issue (because there will be an intervening callvirt that will avoid the decompose bug)
https://sharplab.io/#v2:C4LglgNgPgAgTARgLACgDEA7ArhCBDAIwgFMACAEzAGdCTVSHTUYBmU+UgYVIG97HWpAEpYMwMAFtiAFQCeAB2IAJPBnIlSAWQAUASl79GAgOylgC4gHsAZtpgIADLoB0cxSrUkA3IdIBfVD8gA=

ie instead of ldtoken System.Runtime]System.String emit

 IL_0000: ldtoken [System.Runtime]System.String
IL_0005: call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle)
IL_000a: callvirt instance valuetype [System.Runtime]System.RuntimeTypeHandle [System.Runtime]System.Type::get_TypeHandle()

@lambdageeklambdageek added this to the 8.0.0 milestone Aug 18, 2023
@lewing

Copy link
Copy Markdown
Member

if this is a blocker for rc1 we should open the backport now and send the email

@lambdageek

Copy link
Copy Markdown
MemberAuthor

/backport to release/8.0-rc1

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/8.0-rc1: https://github.com/dotnet/runtime/actions/runs/5907349234

@lambdageek

lambdageek commented Aug 18, 2023

Copy link
Copy Markdown
MemberAuthor

@lewing I'm offline for at least another 3 hours. Can you send the email - see the comments in the issue for details I sent the email. Waiting on a decision...

@lewing
lewing merged commit 940b332 into dotnet:mainAug 19, 2023
@ghostghost locked as resolved and limited conversation to collaborators Sep 18, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Mono] RuntimeTypeHandle.Equals seems to be broken in Release mode

3 participants

@lambdageek@lewing@SamMonoRT
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

[mini] Fix typo in mono_decompose_vtype_opts - #90825

Merged
lewing merged 2 commits into
dotnet:mainfrom
lambdageek:fix-gh-90800
Aug 19, 2023
Merged

[mini] Fix typo in mono_decompose_vtype_opts#90825
lewing merged 2 commits into
dotnet:mainfrom
lambdageek:fix-gh-90800

Conversation

@lambdageek

@lambdageeklambdageek commented Aug 18, 2023

Copy link
Copy Markdown
Member

Without this, if some previous instruction already created a vreg for ins->dest (for example if we are doing multiple passes over the basic block because restart == TRUE) we will use an incorrect vreg for the src when decomposing the current VMOVE


Additionally, only emit OP_LDTOKEN_FIELD if we loaded a field token

This is used by a CreateSpan intrinsic #81695 that needs access to the MonoClassField*

For other cases of a bare LDTOKEN (such as hand-written IL that calls LDTOKEN on a type but doesn't follow it up with a call to GetTypeFromHandle leave the opcode as a VMOVE (from the EMIT_NEW_TEMPLOAD above))

Fixes#90800

Without this, if some previous instruction already created a vreg for
ins->dest (for example if we are doing multiple passes over the basic
block because `restart == TRUE`) we will use an incorrect vreg when
decomposing the current VMOVE
Fixesdotnet#90800
This is used by a CreateSpan optimization that needs access to the
MonoClassField*
For other cases of a bare LDTOKEN (such as hand-written IL that calls
LDTOKEN on a type but doesn't follow it up with a call to
`GetTypeFromHandle` leave the opcode as a VMOVE (from the
`EMIT_NEW_TEMPLOAD` above))
@lambdageek

Copy link
Copy Markdown
MemberAuthor

TODO: need to add a regression test.

attn @SamMonoRT we will need to backport this to net8.

@lambdageek

Copy link
Copy Markdown
MemberAuthor

@simonrozsival@rolfbjarne do we need to try to get this into rc1, is it completely blocking ios?

As a workaround, emitting roslyn-style IL will avoid this issue (because there will be an intervening callvirt that will avoid the decompose bug)
https://sharplab.io/#v2:C4LglgNgPgAgTARgLACgDEA7ArhCBDAIwgFMACAEzAGdCTVSHTUYBmU+UgYVIG97HWpAEpYMwMAFtiAFQCeAB2IAJPBnIlSAWQAUASl79GAgOylgC4gHsAZtpgIADLoB0cxSrUkA3IdIBfVD8gA=

ie instead of ldtoken System.Runtime]System.String emit

 IL_0000: ldtoken [System.Runtime]System.String
IL_0005: call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle)
IL_000a: callvirt instance valuetype [System.Runtime]System.RuntimeTypeHandle [System.Runtime]System.Type::get_TypeHandle()

@lambdageeklambdageek added this to the 8.0.0 milestone Aug 18, 2023
@lewing

Copy link
Copy Markdown
Member

if this is a blocker for rc1 we should open the backport now and send the email

@lambdageek

Copy link
Copy Markdown
MemberAuthor

/backport to release/8.0-rc1

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/8.0-rc1: https://github.com/dotnet/runtime/actions/runs/5907349234

@lambdageek

lambdageek commented Aug 18, 2023

Copy link
Copy Markdown
MemberAuthor

@lewing I'm offline for at least another 3 hours. Can you send the email - see the comments in the issue for details I sent the email. Waiting on a decision...

@lewing
lewing merged commit 940b332 into dotnet:mainAug 19, 2023
@ghostghost locked as resolved and limited conversation to collaborators Sep 18, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Mono] RuntimeTypeHandle.Equals seems to be broken in Release mode

3 participants

@lambdageek@lewing@SamMonoRT
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

[mini] Fix typo in mono_decompose_vtype_opts - #90825

Merged
lewing merged 2 commits into
dotnet:mainfrom
lambdageek:fix-gh-90800
Aug 19, 2023
Merged

[mini] Fix typo in mono_decompose_vtype_opts#90825
lewing merged 2 commits into
dotnet:mainfrom
lambdageek:fix-gh-90800

Conversation

@lambdageek

@lambdageeklambdageek commented Aug 18, 2023

Copy link
Copy Markdown
Member

Without this, if some previous instruction already created a vreg for ins->dest (for example if we are doing multiple passes over the basic block because restart == TRUE) we will use an incorrect vreg for the src when decomposing the current VMOVE


Additionally, only emit OP_LDTOKEN_FIELD if we loaded a field token

This is used by a CreateSpan intrinsic #81695 that needs access to the MonoClassField*

For other cases of a bare LDTOKEN (such as hand-written IL that calls LDTOKEN on a type but doesn't follow it up with a call to GetTypeFromHandle leave the opcode as a VMOVE (from the EMIT_NEW_TEMPLOAD above))

Fixes#90800

Without this, if some previous instruction already created a vreg for
ins->dest (for example if we are doing multiple passes over the basic
block because `restart == TRUE`) we will use an incorrect vreg when
decomposing the current VMOVE
Fixesdotnet#90800
This is used by a CreateSpan optimization that needs access to the
MonoClassField*
For other cases of a bare LDTOKEN (such as hand-written IL that calls
LDTOKEN on a type but doesn't follow it up with a call to
`GetTypeFromHandle` leave the opcode as a VMOVE (from the
`EMIT_NEW_TEMPLOAD` above))
@lambdageek

Copy link
Copy Markdown
MemberAuthor

TODO: need to add a regression test.

attn @SamMonoRT we will need to backport this to net8.

@lambdageek

Copy link
Copy Markdown
MemberAuthor

@simonrozsival@rolfbjarne do we need to try to get this into rc1, is it completely blocking ios?

As a workaround, emitting roslyn-style IL will avoid this issue (because there will be an intervening callvirt that will avoid the decompose bug)
https://sharplab.io/#v2:C4LglgNgPgAgTARgLACgDEA7ArhCBDAIwgFMACAEzAGdCTVSHTUYBmU+UgYVIG97HWpAEpYMwMAFtiAFQCeAB2IAJPBnIlSAWQAUASl79GAgOylgC4gHsAZtpgIADLoB0cxSrUkA3IdIBfVD8gA=

ie instead of ldtoken System.Runtime]System.String emit

 IL_0000: ldtoken [System.Runtime]System.String
IL_0005: call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle)
IL_000a: callvirt instance valuetype [System.Runtime]System.RuntimeTypeHandle [System.Runtime]System.Type::get_TypeHandle()

@lambdageeklambdageek added this to the 8.0.0 milestone Aug 18, 2023
@lewing

Copy link
Copy Markdown
Member

if this is a blocker for rc1 we should open the backport now and send the email

@lambdageek

Copy link
Copy Markdown
MemberAuthor

/backport to release/8.0-rc1

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/8.0-rc1: https://github.com/dotnet/runtime/actions/runs/5907349234

@lambdageek

lambdageek commented Aug 18, 2023

Copy link
Copy Markdown
MemberAuthor

@lewing I'm offline for at least another 3 hours. Can you send the email - see the comments in the issue for details I sent the email. Waiting on a decision...

@lewing
lewing merged commit 940b332 into dotnet:mainAug 19, 2023
@ghostghost locked as resolved and limited conversation to collaborators Sep 18, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Mono] RuntimeTypeHandle.Equals seems to be broken in Release mode

3 participants

@lambdageek@lewing@SamMonoRT
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

[mini] Fix typo in mono_decompose_vtype_opts - #90825

Merged
lewing merged 2 commits into
dotnet:mainfrom
lambdageek:fix-gh-90800
Aug 19, 2023
Merged

[mini] Fix typo in mono_decompose_vtype_opts#90825
lewing merged 2 commits into
dotnet:mainfrom
lambdageek:fix-gh-90800

Conversation

@lambdageek

@lambdageeklambdageek commented Aug 18, 2023

Copy link
Copy Markdown
Member

Without this, if some previous instruction already created a vreg for ins->dest (for example if we are doing multiple passes over the basic block because restart == TRUE) we will use an incorrect vreg for the src when decomposing the current VMOVE


Additionally, only emit OP_LDTOKEN_FIELD if we loaded a field token

This is used by a CreateSpan intrinsic #81695 that needs access to the MonoClassField*

For other cases of a bare LDTOKEN (such as hand-written IL that calls LDTOKEN on a type but doesn't follow it up with a call to GetTypeFromHandle leave the opcode as a VMOVE (from the EMIT_NEW_TEMPLOAD above))

Fixes#90800

Without this, if some previous instruction already created a vreg for
ins->dest (for example if we are doing multiple passes over the basic
block because `restart == TRUE`) we will use an incorrect vreg when
decomposing the current VMOVE
Fixesdotnet#90800
This is used by a CreateSpan optimization that needs access to the
MonoClassField*
For other cases of a bare LDTOKEN (such as hand-written IL that calls
LDTOKEN on a type but doesn't follow it up with a call to
`GetTypeFromHandle` leave the opcode as a VMOVE (from the
`EMIT_NEW_TEMPLOAD` above))
@lambdageek

Copy link
Copy Markdown
MemberAuthor

TODO: need to add a regression test.

attn @SamMonoRT we will need to backport this to net8.

@lambdageek

Copy link
Copy Markdown
MemberAuthor

@simonrozsival@rolfbjarne do we need to try to get this into rc1, is it completely blocking ios?

As a workaround, emitting roslyn-style IL will avoid this issue (because there will be an intervening callvirt that will avoid the decompose bug)
https://sharplab.io/#v2:C4LglgNgPgAgTARgLACgDEA7ArhCBDAIwgFMACAEzAGdCTVSHTUYBmU+UgYVIG97HWpAEpYMwMAFtiAFQCeAB2IAJPBnIlSAWQAUASl79GAgOylgC4gHsAZtpgIADLoB0cxSrUkA3IdIBfVD8gA=

ie instead of ldtoken System.Runtime]System.String emit

 IL_0000: ldtoken [System.Runtime]System.String
IL_0005: call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle)
IL_000a: callvirt instance valuetype [System.Runtime]System.RuntimeTypeHandle [System.Runtime]System.Type::get_TypeHandle()

@lambdageeklambdageek added this to the 8.0.0 milestone Aug 18, 2023
@lewing

Copy link
Copy Markdown
Member

if this is a blocker for rc1 we should open the backport now and send the email

@lambdageek

Copy link
Copy Markdown
MemberAuthor

/backport to release/8.0-rc1

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/8.0-rc1: https://github.com/dotnet/runtime/actions/runs/5907349234

@lambdageek

lambdageek commented Aug 18, 2023

Copy link
Copy Markdown
MemberAuthor

@lewing I'm offline for at least another 3 hours. Can you send the email - see the comments in the issue for details I sent the email. Waiting on a decision...

@lewing
lewing merged commit 940b332 into dotnet:mainAug 19, 2023
@ghostghost locked as resolved and limited conversation to collaborators Sep 18, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Mono] RuntimeTypeHandle.Equals seems to be broken in Release mode

3 participants

@lambdageek@lewing@SamMonoRT
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

[mini] Fix typo in mono_decompose_vtype_opts - #90825

Merged
lewing merged 2 commits into
dotnet:mainfrom
lambdageek:fix-gh-90800
Aug 19, 2023
Merged

[mini] Fix typo in mono_decompose_vtype_opts#90825
lewing merged 2 commits into
dotnet:mainfrom
lambdageek:fix-gh-90800

Conversation

@lambdageek

@lambdageeklambdageek commented Aug 18, 2023

Copy link
Copy Markdown
Member

Without this, if some previous instruction already created a vreg for ins->dest (for example if we are doing multiple passes over the basic block because restart == TRUE) we will use an incorrect vreg for the src when decomposing the current VMOVE


Additionally, only emit OP_LDTOKEN_FIELD if we loaded a field token

This is used by a CreateSpan intrinsic #81695 that needs access to the MonoClassField*

For other cases of a bare LDTOKEN (such as hand-written IL that calls LDTOKEN on a type but doesn't follow it up with a call to GetTypeFromHandle leave the opcode as a VMOVE (from the EMIT_NEW_TEMPLOAD above))

Fixes#90800

Without this, if some previous instruction already created a vreg for
ins->dest (for example if we are doing multiple passes over the basic
block because `restart == TRUE`) we will use an incorrect vreg when
decomposing the current VMOVE
Fixesdotnet#90800
This is used by a CreateSpan optimization that needs access to the
MonoClassField*
For other cases of a bare LDTOKEN (such as hand-written IL that calls
LDTOKEN on a type but doesn't follow it up with a call to
`GetTypeFromHandle` leave the opcode as a VMOVE (from the
`EMIT_NEW_TEMPLOAD` above))
@lambdageek

Copy link
Copy Markdown
MemberAuthor

TODO: need to add a regression test.

attn @SamMonoRT we will need to backport this to net8.

@lambdageek

Copy link
Copy Markdown
MemberAuthor

@simonrozsival@rolfbjarne do we need to try to get this into rc1, is it completely blocking ios?

As a workaround, emitting roslyn-style IL will avoid this issue (because there will be an intervening callvirt that will avoid the decompose bug)
https://sharplab.io/#v2:C4LglgNgPgAgTARgLACgDEA7ArhCBDAIwgFMACAEzAGdCTVSHTUYBmU+UgYVIG97HWpAEpYMwMAFtiAFQCeAB2IAJPBnIlSAWQAUASl79GAgOylgC4gHsAZtpgIADLoB0cxSrUkA3IdIBfVD8gA=

ie instead of ldtoken System.Runtime]System.String emit

 IL_0000: ldtoken [System.Runtime]System.String
IL_0005: call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle)
IL_000a: callvirt instance valuetype [System.Runtime]System.RuntimeTypeHandle [System.Runtime]System.Type::get_TypeHandle()

@lambdageeklambdageek added this to the 8.0.0 milestone Aug 18, 2023
@lewing

Copy link
Copy Markdown
Member

if this is a blocker for rc1 we should open the backport now and send the email

@lambdageek

Copy link
Copy Markdown
MemberAuthor

/backport to release/8.0-rc1

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/8.0-rc1: https://github.com/dotnet/runtime/actions/runs/5907349234

@lambdageek

lambdageek commented Aug 18, 2023

Copy link
Copy Markdown
MemberAuthor

@lewing I'm offline for at least another 3 hours. Can you send the email - see the comments in the issue for details I sent the email. Waiting on a decision...

@lewing
lewing merged commit 940b332 into dotnet:mainAug 19, 2023
@ghostghost locked as resolved and limited conversation to collaborators Sep 18, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Mono] RuntimeTypeHandle.Equals seems to be broken in Release mode

3 participants

@lambdageek@lewing@SamMonoRT
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

[mini] Fix typo in mono_decompose_vtype_opts - #90825

Merged
lewing merged 2 commits into
dotnet:mainfrom
lambdageek:fix-gh-90800
Aug 19, 2023
Merged

[mini] Fix typo in mono_decompose_vtype_opts#90825
lewing merged 2 commits into
dotnet:mainfrom
lambdageek:fix-gh-90800

Conversation

@lambdageek

@lambdageeklambdageek commented Aug 18, 2023

Copy link
Copy Markdown
Member

Without this, if some previous instruction already created a vreg for ins->dest (for example if we are doing multiple passes over the basic block because restart == TRUE) we will use an incorrect vreg for the src when decomposing the current VMOVE


Additionally, only emit OP_LDTOKEN_FIELD if we loaded a field token

This is used by a CreateSpan intrinsic #81695 that needs access to the MonoClassField*

For other cases of a bare LDTOKEN (such as hand-written IL that calls LDTOKEN on a type but doesn't follow it up with a call to GetTypeFromHandle leave the opcode as a VMOVE (from the EMIT_NEW_TEMPLOAD above))

Fixes#90800

Without this, if some previous instruction already created a vreg for
ins->dest (for example if we are doing multiple passes over the basic
block because `restart == TRUE`) we will use an incorrect vreg when
decomposing the current VMOVE
Fixesdotnet#90800
This is used by a CreateSpan optimization that needs access to the
MonoClassField*
For other cases of a bare LDTOKEN (such as hand-written IL that calls
LDTOKEN on a type but doesn't follow it up with a call to
`GetTypeFromHandle` leave the opcode as a VMOVE (from the
`EMIT_NEW_TEMPLOAD` above))
@lambdageek

Copy link
Copy Markdown
MemberAuthor

TODO: need to add a regression test.

attn @SamMonoRT we will need to backport this to net8.

@lambdageek

Copy link
Copy Markdown
MemberAuthor

@simonrozsival@rolfbjarne do we need to try to get this into rc1, is it completely blocking ios?

As a workaround, emitting roslyn-style IL will avoid this issue (because there will be an intervening callvirt that will avoid the decompose bug)
https://sharplab.io/#v2:C4LglgNgPgAgTARgLACgDEA7ArhCBDAIwgFMACAEzAGdCTVSHTUYBmU+UgYVIG97HWpAEpYMwMAFtiAFQCeAB2IAJPBnIlSAWQAUASl79GAgOylgC4gHsAZtpgIADLoB0cxSrUkA3IdIBfVD8gA=

ie instead of ldtoken System.Runtime]System.String emit

 IL_0000: ldtoken [System.Runtime]System.String
IL_0005: call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle)
IL_000a: callvirt instance valuetype [System.Runtime]System.RuntimeTypeHandle [System.Runtime]System.Type::get_TypeHandle()

@lambdageeklambdageek added this to the 8.0.0 milestone Aug 18, 2023
@lewing

Copy link
Copy Markdown
Member

if this is a blocker for rc1 we should open the backport now and send the email

@lambdageek

Copy link
Copy Markdown
MemberAuthor

/backport to release/8.0-rc1

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/8.0-rc1: https://github.com/dotnet/runtime/actions/runs/5907349234

@lambdageek

lambdageek commented Aug 18, 2023

Copy link
Copy Markdown
MemberAuthor

@lewing I'm offline for at least another 3 hours. Can you send the email - see the comments in the issue for details I sent the email. Waiting on a decision...

@lewing
lewing merged commit 940b332 into dotnet:mainAug 19, 2023
@ghostghost locked as resolved and limited conversation to collaborators Sep 18, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Mono] RuntimeTypeHandle.Equals seems to be broken in Release mode

3 participants

@lambdageek@lewing@SamMonoRT