JIT: Add a uniform representation for parameter ABI information - #100138

Merged
jakobbotsch merged 9 commits into
dotnet:mainfrom
jakobbotsch:refactor-abi
Mar 25, 2024
Merged

JIT: Add a uniform representation for parameter ABI information#100138
jakobbotsch merged 9 commits into
dotnet:mainfrom
jakobbotsch:refactor-abi

Conversation

@jakobbotsch

Copy link
Copy Markdown
Member

This adds a uniform representation that can represent the ABI
information for all of our targets without needing to fall back to
handling ABI specific details in all places that need to handle calling
conventions.

Currently nothing is using this information. I want to incrementally
migrate our ABI handling to use this representation. Also, there are
several potential future improvements:

  • Split out ABI classification per ABI instead of keeping them all
    within the same function
  • Unify InitVarDscInfo::stackArgSize and InitVarDscInfo::argSize. I
    am unsure why the latter is needed
  • Remove LclVarDsc::GetArgReg(), LclVarDscInfo::GetOtherArgReg(),
    HFA related members
  • Reuse the representation in CallArgABIInformation and unify the
    classifiers

The end goal here is rewriting genFnPrologCalleeRegArgs to handle
float and integer registers simultaneously, and to support some of the
registers that the Swift calling convention is using.

This adds a uniform representation that can represent the ABI
information for all of our targets without needing to fall back to
handling ABI specific details in all places that need to handle calling
conventions.
Currently nothing is using this information. I want to incrementally
migrate our ABI handling to use this representation. Also, there are
several potential future improvements:
- Split out ABI classification per ABI instead of keeping them all
within the same function
- Unify `InitVarDscInfo::stackArgSize` and `InitVarDscInfo::argSize`. I
am unsure why the latter is needed
- Remove `LclVarDsc::GetArgReg()`, `LclVarDscInfo::GetOtherArgReg()`,
HFA related members
- Reuse the representation in `CallArgABIInformation` and unify the
classifiers
The end goal here is rewriting `genFnPrologCalleeRegArgs` to handle
float and integer registers simultaneously, and to support some of the
registers that the Swift calling convention is using.
@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 22, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@jakobbotsch
jakobbotsch marked this pull request as ready for review March 25, 2024 10:35
@jakobbotsch

Copy link
Copy Markdown
MemberAuthor

cc @dotnet/jit-contrib PTAL @AndyAyersMS

No diffs. Some minor TP regressions (the seemingly large MinOpts ones are in collections with < 10 MinOpts contexts).

There's a lot more clean up to be done based on this, but I initially want to use this to represent the ABI details for the structs in Swift reverse pinvokes.

Comment threadsrc/coreclr/jit/abi.h
Comment on lines +12 to +13
bool IsPassedInRegister() const;
bool IsPassedOnStack() const;

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.

Is it relevant to track the other common qualifications like HFA and HVA?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I don't think so. HFA's/HVA's are just passed in more registers than other struct arguments, so hopefully by the end of this clean up that's a detail that's constrained fully to be within the ABI classification and not leaked out anywhere to the rest of the JIT.

Comment threadsrc/coreclr/jit/abi.h
Comment on lines +43 to +44
// - On arm64/arm32, HFAs can be passed in up to four registers, giving
// four register segments

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.

This should also include HVAs, which looks like we might be missing handling around:

godbolt HFA/HVA for Arm64

godbolt HVA/HVA for x64

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

These are just examples to indicate examples of the representation. The intention of this PR is not to make any functional changes.

@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.

Seems like a good start...

@jakobbotsch
jakobbotsch merged commit 13a9036 into dotnet:mainMar 25, 2024
@jakobbotsch
jakobbotsch deleted the refactor-abi branch March 25, 2024 17:34
jakobbotsch added a commit that referenced this pull request Apr 5, 2024
This adds the final support for frozen structs in UCO methods.
This passes 2500 auto generated tests locally on both macOS x64 and arm64. This
PR includes only 100 tests.
Frozen struct parameters are handled via the new ABI representation added in
#100138. When such a parameter exists we always allocate space for it on the
local stack frame. The struct is then reassembled from its passed constituents
as the first thing in the codegen.
One complication is that there can be an arbitrary amount of codegen to handle
this reassembling. We cannot handle an arbitrary amount of codegen in the
prolog, so the reassembling is handled in two places. First, since the amount of
register passed data is limited, we handle those in the prolog (which frees them
up to be used for other things). If some pieces were passed on the stack the JIT
then ensures that there is a scratch BB and generates the code to reassemble the
remaining parts as the first thing in the scratch BB.
Since Swift structs can be passed by reference in certain cases this PR also
enables `FEATURE_IMPLICIT_BYREFS` for SysV x64 to handle those cases. Depending
on the TP impact we can refine some of the ifdefs around this.
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 25, 2024
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 SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jakobbotsch@AndyAyersMS@tannergooding
, '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

JIT: Add a uniform representation for parameter ABI information - #100138

Merged
jakobbotsch merged 9 commits into
dotnet:mainfrom
jakobbotsch:refactor-abi
Mar 25, 2024
Merged

JIT: Add a uniform representation for parameter ABI information#100138
jakobbotsch merged 9 commits into
dotnet:mainfrom
jakobbotsch:refactor-abi

Conversation

@jakobbotsch

Copy link
Copy Markdown
Member

This adds a uniform representation that can represent the ABI
information for all of our targets without needing to fall back to
handling ABI specific details in all places that need to handle calling
conventions.

Currently nothing is using this information. I want to incrementally
migrate our ABI handling to use this representation. Also, there are
several potential future improvements:

  • Split out ABI classification per ABI instead of keeping them all
    within the same function
  • Unify InitVarDscInfo::stackArgSize and InitVarDscInfo::argSize. I
    am unsure why the latter is needed
  • Remove LclVarDsc::GetArgReg(), LclVarDscInfo::GetOtherArgReg(),
    HFA related members
  • Reuse the representation in CallArgABIInformation and unify the
    classifiers

The end goal here is rewriting genFnPrologCalleeRegArgs to handle
float and integer registers simultaneously, and to support some of the
registers that the Swift calling convention is using.

This adds a uniform representation that can represent the ABI
information for all of our targets without needing to fall back to
handling ABI specific details in all places that need to handle calling
conventions.
Currently nothing is using this information. I want to incrementally
migrate our ABI handling to use this representation. Also, there are
several potential future improvements:
- Split out ABI classification per ABI instead of keeping them all
within the same function
- Unify `InitVarDscInfo::stackArgSize` and `InitVarDscInfo::argSize`. I
am unsure why the latter is needed
- Remove `LclVarDsc::GetArgReg()`, `LclVarDscInfo::GetOtherArgReg()`,
HFA related members
- Reuse the representation in `CallArgABIInformation` and unify the
classifiers
The end goal here is rewriting `genFnPrologCalleeRegArgs` to handle
float and integer registers simultaneously, and to support some of the
registers that the Swift calling convention is using.
@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 22, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@jakobbotsch
jakobbotsch marked this pull request as ready for review March 25, 2024 10:35
@jakobbotsch

Copy link
Copy Markdown
MemberAuthor

cc @dotnet/jit-contrib PTAL @AndyAyersMS

No diffs. Some minor TP regressions (the seemingly large MinOpts ones are in collections with < 10 MinOpts contexts).

There's a lot more clean up to be done based on this, but I initially want to use this to represent the ABI details for the structs in Swift reverse pinvokes.

Comment threadsrc/coreclr/jit/abi.h
Comment on lines +12 to +13
bool IsPassedInRegister() const;
bool IsPassedOnStack() const;

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.

Is it relevant to track the other common qualifications like HFA and HVA?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I don't think so. HFA's/HVA's are just passed in more registers than other struct arguments, so hopefully by the end of this clean up that's a detail that's constrained fully to be within the ABI classification and not leaked out anywhere to the rest of the JIT.

Comment threadsrc/coreclr/jit/abi.h
Comment on lines +43 to +44
// - On arm64/arm32, HFAs can be passed in up to four registers, giving
// four register segments

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.

This should also include HVAs, which looks like we might be missing handling around:

godbolt HFA/HVA for Arm64

godbolt HVA/HVA for x64

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

These are just examples to indicate examples of the representation. The intention of this PR is not to make any functional changes.

@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.

Seems like a good start...

@jakobbotsch
jakobbotsch merged commit 13a9036 into dotnet:mainMar 25, 2024
@jakobbotsch
jakobbotsch deleted the refactor-abi branch March 25, 2024 17:34
jakobbotsch added a commit that referenced this pull request Apr 5, 2024
This adds the final support for frozen structs in UCO methods.
This passes 2500 auto generated tests locally on both macOS x64 and arm64. This
PR includes only 100 tests.
Frozen struct parameters are handled via the new ABI representation added in
#100138. When such a parameter exists we always allocate space for it on the
local stack frame. The struct is then reassembled from its passed constituents
as the first thing in the codegen.
One complication is that there can be an arbitrary amount of codegen to handle
this reassembling. We cannot handle an arbitrary amount of codegen in the
prolog, so the reassembling is handled in two places. First, since the amount of
register passed data is limited, we handle those in the prolog (which frees them
up to be used for other things). If some pieces were passed on the stack the JIT
then ensures that there is a scratch BB and generates the code to reassemble the
remaining parts as the first thing in the scratch BB.
Since Swift structs can be passed by reference in certain cases this PR also
enables `FEATURE_IMPLICIT_BYREFS` for SysV x64 to handle those cases. Depending
on the TP impact we can refine some of the ifdefs around this.
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 25, 2024
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 SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jakobbotsch@AndyAyersMS@tannergooding
, '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

JIT: Add a uniform representation for parameter ABI information - #100138

Merged
jakobbotsch merged 9 commits into
dotnet:mainfrom
jakobbotsch:refactor-abi
Mar 25, 2024
Merged

JIT: Add a uniform representation for parameter ABI information#100138
jakobbotsch merged 9 commits into
dotnet:mainfrom
jakobbotsch:refactor-abi

Conversation

@jakobbotsch

Copy link
Copy Markdown
Member

This adds a uniform representation that can represent the ABI
information for all of our targets without needing to fall back to
handling ABI specific details in all places that need to handle calling
conventions.

Currently nothing is using this information. I want to incrementally
migrate our ABI handling to use this representation. Also, there are
several potential future improvements:

  • Split out ABI classification per ABI instead of keeping them all
    within the same function
  • Unify InitVarDscInfo::stackArgSize and InitVarDscInfo::argSize. I
    am unsure why the latter is needed
  • Remove LclVarDsc::GetArgReg(), LclVarDscInfo::GetOtherArgReg(),
    HFA related members
  • Reuse the representation in CallArgABIInformation and unify the
    classifiers

The end goal here is rewriting genFnPrologCalleeRegArgs to handle
float and integer registers simultaneously, and to support some of the
registers that the Swift calling convention is using.

This adds a uniform representation that can represent the ABI
information for all of our targets without needing to fall back to
handling ABI specific details in all places that need to handle calling
conventions.
Currently nothing is using this information. I want to incrementally
migrate our ABI handling to use this representation. Also, there are
several potential future improvements:
- Split out ABI classification per ABI instead of keeping them all
within the same function
- Unify `InitVarDscInfo::stackArgSize` and `InitVarDscInfo::argSize`. I
am unsure why the latter is needed
- Remove `LclVarDsc::GetArgReg()`, `LclVarDscInfo::GetOtherArgReg()`,
HFA related members
- Reuse the representation in `CallArgABIInformation` and unify the
classifiers
The end goal here is rewriting `genFnPrologCalleeRegArgs` to handle
float and integer registers simultaneously, and to support some of the
registers that the Swift calling convention is using.
@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 22, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@jakobbotsch
jakobbotsch marked this pull request as ready for review March 25, 2024 10:35
@jakobbotsch

Copy link
Copy Markdown
MemberAuthor

cc @dotnet/jit-contrib PTAL @AndyAyersMS

No diffs. Some minor TP regressions (the seemingly large MinOpts ones are in collections with < 10 MinOpts contexts).

There's a lot more clean up to be done based on this, but I initially want to use this to represent the ABI details for the structs in Swift reverse pinvokes.

Comment threadsrc/coreclr/jit/abi.h
Comment on lines +12 to +13
bool IsPassedInRegister() const;
bool IsPassedOnStack() const;

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.

Is it relevant to track the other common qualifications like HFA and HVA?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I don't think so. HFA's/HVA's are just passed in more registers than other struct arguments, so hopefully by the end of this clean up that's a detail that's constrained fully to be within the ABI classification and not leaked out anywhere to the rest of the JIT.

Comment threadsrc/coreclr/jit/abi.h
Comment on lines +43 to +44
// - On arm64/arm32, HFAs can be passed in up to four registers, giving
// four register segments

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.

This should also include HVAs, which looks like we might be missing handling around:

godbolt HFA/HVA for Arm64

godbolt HVA/HVA for x64

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

These are just examples to indicate examples of the representation. The intention of this PR is not to make any functional changes.

@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.

Seems like a good start...

@jakobbotsch
jakobbotsch merged commit 13a9036 into dotnet:mainMar 25, 2024
@jakobbotsch
jakobbotsch deleted the refactor-abi branch March 25, 2024 17:34
jakobbotsch added a commit that referenced this pull request Apr 5, 2024
This adds the final support for frozen structs in UCO methods.
This passes 2500 auto generated tests locally on both macOS x64 and arm64. This
PR includes only 100 tests.
Frozen struct parameters are handled via the new ABI representation added in
#100138. When such a parameter exists we always allocate space for it on the
local stack frame. The struct is then reassembled from its passed constituents
as the first thing in the codegen.
One complication is that there can be an arbitrary amount of codegen to handle
this reassembling. We cannot handle an arbitrary amount of codegen in the
prolog, so the reassembling is handled in two places. First, since the amount of
register passed data is limited, we handle those in the prolog (which frees them
up to be used for other things). If some pieces were passed on the stack the JIT
then ensures that there is a scratch BB and generates the code to reassemble the
remaining parts as the first thing in the scratch BB.
Since Swift structs can be passed by reference in certain cases this PR also
enables `FEATURE_IMPLICIT_BYREFS` for SysV x64 to handle those cases. Depending
on the TP impact we can refine some of the ifdefs around this.
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 25, 2024
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 SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jakobbotsch@AndyAyersMS@tannergooding
, '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

JIT: Add a uniform representation for parameter ABI information - #100138

Merged
jakobbotsch merged 9 commits into
dotnet:mainfrom
jakobbotsch:refactor-abi
Mar 25, 2024
Merged

JIT: Add a uniform representation for parameter ABI information#100138
jakobbotsch merged 9 commits into
dotnet:mainfrom
jakobbotsch:refactor-abi

Conversation

@jakobbotsch

Copy link
Copy Markdown
Member

This adds a uniform representation that can represent the ABI
information for all of our targets without needing to fall back to
handling ABI specific details in all places that need to handle calling
conventions.

Currently nothing is using this information. I want to incrementally
migrate our ABI handling to use this representation. Also, there are
several potential future improvements:

  • Split out ABI classification per ABI instead of keeping them all
    within the same function
  • Unify InitVarDscInfo::stackArgSize and InitVarDscInfo::argSize. I
    am unsure why the latter is needed
  • Remove LclVarDsc::GetArgReg(), LclVarDscInfo::GetOtherArgReg(),
    HFA related members
  • Reuse the representation in CallArgABIInformation and unify the
    classifiers

The end goal here is rewriting genFnPrologCalleeRegArgs to handle
float and integer registers simultaneously, and to support some of the
registers that the Swift calling convention is using.

This adds a uniform representation that can represent the ABI
information for all of our targets without needing to fall back to
handling ABI specific details in all places that need to handle calling
conventions.
Currently nothing is using this information. I want to incrementally
migrate our ABI handling to use this representation. Also, there are
several potential future improvements:
- Split out ABI classification per ABI instead of keeping them all
within the same function
- Unify `InitVarDscInfo::stackArgSize` and `InitVarDscInfo::argSize`. I
am unsure why the latter is needed
- Remove `LclVarDsc::GetArgReg()`, `LclVarDscInfo::GetOtherArgReg()`,
HFA related members
- Reuse the representation in `CallArgABIInformation` and unify the
classifiers
The end goal here is rewriting `genFnPrologCalleeRegArgs` to handle
float and integer registers simultaneously, and to support some of the
registers that the Swift calling convention is using.
@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 22, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@jakobbotsch
jakobbotsch marked this pull request as ready for review March 25, 2024 10:35
@jakobbotsch

Copy link
Copy Markdown
MemberAuthor

cc @dotnet/jit-contrib PTAL @AndyAyersMS

No diffs. Some minor TP regressions (the seemingly large MinOpts ones are in collections with < 10 MinOpts contexts).

There's a lot more clean up to be done based on this, but I initially want to use this to represent the ABI details for the structs in Swift reverse pinvokes.

Comment threadsrc/coreclr/jit/abi.h
Comment on lines +12 to +13
bool IsPassedInRegister() const;
bool IsPassedOnStack() const;

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.

Is it relevant to track the other common qualifications like HFA and HVA?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I don't think so. HFA's/HVA's are just passed in more registers than other struct arguments, so hopefully by the end of this clean up that's a detail that's constrained fully to be within the ABI classification and not leaked out anywhere to the rest of the JIT.

Comment threadsrc/coreclr/jit/abi.h
Comment on lines +43 to +44
// - On arm64/arm32, HFAs can be passed in up to four registers, giving
// four register segments

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.

This should also include HVAs, which looks like we might be missing handling around:

godbolt HFA/HVA for Arm64

godbolt HVA/HVA for x64

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

These are just examples to indicate examples of the representation. The intention of this PR is not to make any functional changes.

@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.

Seems like a good start...

@jakobbotsch
jakobbotsch merged commit 13a9036 into dotnet:mainMar 25, 2024
@jakobbotsch
jakobbotsch deleted the refactor-abi branch March 25, 2024 17:34
jakobbotsch added a commit that referenced this pull request Apr 5, 2024
This adds the final support for frozen structs in UCO methods.
This passes 2500 auto generated tests locally on both macOS x64 and arm64. This
PR includes only 100 tests.
Frozen struct parameters are handled via the new ABI representation added in
#100138. When such a parameter exists we always allocate space for it on the
local stack frame. The struct is then reassembled from its passed constituents
as the first thing in the codegen.
One complication is that there can be an arbitrary amount of codegen to handle
this reassembling. We cannot handle an arbitrary amount of codegen in the
prolog, so the reassembling is handled in two places. First, since the amount of
register passed data is limited, we handle those in the prolog (which frees them
up to be used for other things). If some pieces were passed on the stack the JIT
then ensures that there is a scratch BB and generates the code to reassemble the
remaining parts as the first thing in the scratch BB.
Since Swift structs can be passed by reference in certain cases this PR also
enables `FEATURE_IMPLICIT_BYREFS` for SysV x64 to handle those cases. Depending
on the TP impact we can refine some of the ifdefs around this.
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 25, 2024
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 SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jakobbotsch@AndyAyersMS@tannergooding
, '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

JIT: Add a uniform representation for parameter ABI information - #100138

Merged
jakobbotsch merged 9 commits into
dotnet:mainfrom
jakobbotsch:refactor-abi
Mar 25, 2024
Merged

JIT: Add a uniform representation for parameter ABI information#100138
jakobbotsch merged 9 commits into
dotnet:mainfrom
jakobbotsch:refactor-abi

Conversation

@jakobbotsch

Copy link
Copy Markdown
Member

This adds a uniform representation that can represent the ABI
information for all of our targets without needing to fall back to
handling ABI specific details in all places that need to handle calling
conventions.

Currently nothing is using this information. I want to incrementally
migrate our ABI handling to use this representation. Also, there are
several potential future improvements:

  • Split out ABI classification per ABI instead of keeping them all
    within the same function
  • Unify InitVarDscInfo::stackArgSize and InitVarDscInfo::argSize. I
    am unsure why the latter is needed
  • Remove LclVarDsc::GetArgReg(), LclVarDscInfo::GetOtherArgReg(),
    HFA related members
  • Reuse the representation in CallArgABIInformation and unify the
    classifiers

The end goal here is rewriting genFnPrologCalleeRegArgs to handle
float and integer registers simultaneously, and to support some of the
registers that the Swift calling convention is using.

This adds a uniform representation that can represent the ABI
information for all of our targets without needing to fall back to
handling ABI specific details in all places that need to handle calling
conventions.
Currently nothing is using this information. I want to incrementally
migrate our ABI handling to use this representation. Also, there are
several potential future improvements:
- Split out ABI classification per ABI instead of keeping them all
within the same function
- Unify `InitVarDscInfo::stackArgSize` and `InitVarDscInfo::argSize`. I
am unsure why the latter is needed
- Remove `LclVarDsc::GetArgReg()`, `LclVarDscInfo::GetOtherArgReg()`,
HFA related members
- Reuse the representation in `CallArgABIInformation` and unify the
classifiers
The end goal here is rewriting `genFnPrologCalleeRegArgs` to handle
float and integer registers simultaneously, and to support some of the
registers that the Swift calling convention is using.
@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 22, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@jakobbotsch
jakobbotsch marked this pull request as ready for review March 25, 2024 10:35
@jakobbotsch

Copy link
Copy Markdown
MemberAuthor

cc @dotnet/jit-contrib PTAL @AndyAyersMS

No diffs. Some minor TP regressions (the seemingly large MinOpts ones are in collections with < 10 MinOpts contexts).

There's a lot more clean up to be done based on this, but I initially want to use this to represent the ABI details for the structs in Swift reverse pinvokes.

Comment threadsrc/coreclr/jit/abi.h
Comment on lines +12 to +13
bool IsPassedInRegister() const;
bool IsPassedOnStack() const;

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.

Is it relevant to track the other common qualifications like HFA and HVA?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I don't think so. HFA's/HVA's are just passed in more registers than other struct arguments, so hopefully by the end of this clean up that's a detail that's constrained fully to be within the ABI classification and not leaked out anywhere to the rest of the JIT.

Comment threadsrc/coreclr/jit/abi.h
Comment on lines +43 to +44
// - On arm64/arm32, HFAs can be passed in up to four registers, giving
// four register segments

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.

This should also include HVAs, which looks like we might be missing handling around:

godbolt HFA/HVA for Arm64

godbolt HVA/HVA for x64

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

These are just examples to indicate examples of the representation. The intention of this PR is not to make any functional changes.

@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.

Seems like a good start...

@jakobbotsch
jakobbotsch merged commit 13a9036 into dotnet:mainMar 25, 2024
@jakobbotsch
jakobbotsch deleted the refactor-abi branch March 25, 2024 17:34
jakobbotsch added a commit that referenced this pull request Apr 5, 2024
This adds the final support for frozen structs in UCO methods.
This passes 2500 auto generated tests locally on both macOS x64 and arm64. This
PR includes only 100 tests.
Frozen struct parameters are handled via the new ABI representation added in
#100138. When such a parameter exists we always allocate space for it on the
local stack frame. The struct is then reassembled from its passed constituents
as the first thing in the codegen.
One complication is that there can be an arbitrary amount of codegen to handle
this reassembling. We cannot handle an arbitrary amount of codegen in the
prolog, so the reassembling is handled in two places. First, since the amount of
register passed data is limited, we handle those in the prolog (which frees them
up to be used for other things). If some pieces were passed on the stack the JIT
then ensures that there is a scratch BB and generates the code to reassemble the
remaining parts as the first thing in the scratch BB.
Since Swift structs can be passed by reference in certain cases this PR also
enables `FEATURE_IMPLICIT_BYREFS` for SysV x64 to handle those cases. Depending
on the TP impact we can refine some of the ifdefs around this.
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 25, 2024
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 SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jakobbotsch@AndyAyersMS@tannergooding
, '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

JIT: Add a uniform representation for parameter ABI information - #100138

Merged
jakobbotsch merged 9 commits into
dotnet:mainfrom
jakobbotsch:refactor-abi
Mar 25, 2024
Merged

JIT: Add a uniform representation for parameter ABI information#100138
jakobbotsch merged 9 commits into
dotnet:mainfrom
jakobbotsch:refactor-abi

Conversation

@jakobbotsch

Copy link
Copy Markdown
Member

This adds a uniform representation that can represent the ABI
information for all of our targets without needing to fall back to
handling ABI specific details in all places that need to handle calling
conventions.

Currently nothing is using this information. I want to incrementally
migrate our ABI handling to use this representation. Also, there are
several potential future improvements:

  • Split out ABI classification per ABI instead of keeping them all
    within the same function
  • Unify InitVarDscInfo::stackArgSize and InitVarDscInfo::argSize. I
    am unsure why the latter is needed
  • Remove LclVarDsc::GetArgReg(), LclVarDscInfo::GetOtherArgReg(),
    HFA related members
  • Reuse the representation in CallArgABIInformation and unify the
    classifiers

The end goal here is rewriting genFnPrologCalleeRegArgs to handle
float and integer registers simultaneously, and to support some of the
registers that the Swift calling convention is using.

This adds a uniform representation that can represent the ABI
information for all of our targets without needing to fall back to
handling ABI specific details in all places that need to handle calling
conventions.
Currently nothing is using this information. I want to incrementally
migrate our ABI handling to use this representation. Also, there are
several potential future improvements:
- Split out ABI classification per ABI instead of keeping them all
within the same function
- Unify `InitVarDscInfo::stackArgSize` and `InitVarDscInfo::argSize`. I
am unsure why the latter is needed
- Remove `LclVarDsc::GetArgReg()`, `LclVarDscInfo::GetOtherArgReg()`,
HFA related members
- Reuse the representation in `CallArgABIInformation` and unify the
classifiers
The end goal here is rewriting `genFnPrologCalleeRegArgs` to handle
float and integer registers simultaneously, and to support some of the
registers that the Swift calling convention is using.
@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 22, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@jakobbotsch
jakobbotsch marked this pull request as ready for review March 25, 2024 10:35
@jakobbotsch

Copy link
Copy Markdown
MemberAuthor

cc @dotnet/jit-contrib PTAL @AndyAyersMS

No diffs. Some minor TP regressions (the seemingly large MinOpts ones are in collections with < 10 MinOpts contexts).

There's a lot more clean up to be done based on this, but I initially want to use this to represent the ABI details for the structs in Swift reverse pinvokes.

Comment threadsrc/coreclr/jit/abi.h
Comment on lines +12 to +13
bool IsPassedInRegister() const;
bool IsPassedOnStack() const;

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.

Is it relevant to track the other common qualifications like HFA and HVA?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I don't think so. HFA's/HVA's are just passed in more registers than other struct arguments, so hopefully by the end of this clean up that's a detail that's constrained fully to be within the ABI classification and not leaked out anywhere to the rest of the JIT.

Comment threadsrc/coreclr/jit/abi.h
Comment on lines +43 to +44
// - On arm64/arm32, HFAs can be passed in up to four registers, giving
// four register segments

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.

This should also include HVAs, which looks like we might be missing handling around:

godbolt HFA/HVA for Arm64

godbolt HVA/HVA for x64

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

These are just examples to indicate examples of the representation. The intention of this PR is not to make any functional changes.

@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.

Seems like a good start...

@jakobbotsch
jakobbotsch merged commit 13a9036 into dotnet:mainMar 25, 2024
@jakobbotsch
jakobbotsch deleted the refactor-abi branch March 25, 2024 17:34
jakobbotsch added a commit that referenced this pull request Apr 5, 2024
This adds the final support for frozen structs in UCO methods.
This passes 2500 auto generated tests locally on both macOS x64 and arm64. This
PR includes only 100 tests.
Frozen struct parameters are handled via the new ABI representation added in
#100138. When such a parameter exists we always allocate space for it on the
local stack frame. The struct is then reassembled from its passed constituents
as the first thing in the codegen.
One complication is that there can be an arbitrary amount of codegen to handle
this reassembling. We cannot handle an arbitrary amount of codegen in the
prolog, so the reassembling is handled in two places. First, since the amount of
register passed data is limited, we handle those in the prolog (which frees them
up to be used for other things). If some pieces were passed on the stack the JIT
then ensures that there is a scratch BB and generates the code to reassemble the
remaining parts as the first thing in the scratch BB.
Since Swift structs can be passed by reference in certain cases this PR also
enables `FEATURE_IMPLICIT_BYREFS` for SysV x64 to handle those cases. Depending
on the TP impact we can refine some of the ifdefs around this.
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 25, 2024
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 SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jakobbotsch@AndyAyersMS@tannergooding
, '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

JIT: Add a uniform representation for parameter ABI information - #100138

Merged
jakobbotsch merged 9 commits into
dotnet:mainfrom
jakobbotsch:refactor-abi
Mar 25, 2024
Merged

JIT: Add a uniform representation for parameter ABI information#100138
jakobbotsch merged 9 commits into
dotnet:mainfrom
jakobbotsch:refactor-abi

Conversation

@jakobbotsch

Copy link
Copy Markdown
Member

This adds a uniform representation that can represent the ABI
information for all of our targets without needing to fall back to
handling ABI specific details in all places that need to handle calling
conventions.

Currently nothing is using this information. I want to incrementally
migrate our ABI handling to use this representation. Also, there are
several potential future improvements:

  • Split out ABI classification per ABI instead of keeping them all
    within the same function
  • Unify InitVarDscInfo::stackArgSize and InitVarDscInfo::argSize. I
    am unsure why the latter is needed
  • Remove LclVarDsc::GetArgReg(), LclVarDscInfo::GetOtherArgReg(),
    HFA related members
  • Reuse the representation in CallArgABIInformation and unify the
    classifiers

The end goal here is rewriting genFnPrologCalleeRegArgs to handle
float and integer registers simultaneously, and to support some of the
registers that the Swift calling convention is using.

This adds a uniform representation that can represent the ABI
information for all of our targets without needing to fall back to
handling ABI specific details in all places that need to handle calling
conventions.
Currently nothing is using this information. I want to incrementally
migrate our ABI handling to use this representation. Also, there are
several potential future improvements:
- Split out ABI classification per ABI instead of keeping them all
within the same function
- Unify `InitVarDscInfo::stackArgSize` and `InitVarDscInfo::argSize`. I
am unsure why the latter is needed
- Remove `LclVarDsc::GetArgReg()`, `LclVarDscInfo::GetOtherArgReg()`,
HFA related members
- Reuse the representation in `CallArgABIInformation` and unify the
classifiers
The end goal here is rewriting `genFnPrologCalleeRegArgs` to handle
float and integer registers simultaneously, and to support some of the
registers that the Swift calling convention is using.
@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 22, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@jakobbotsch
jakobbotsch marked this pull request as ready for review March 25, 2024 10:35
@jakobbotsch

Copy link
Copy Markdown
MemberAuthor

cc @dotnet/jit-contrib PTAL @AndyAyersMS

No diffs. Some minor TP regressions (the seemingly large MinOpts ones are in collections with < 10 MinOpts contexts).

There's a lot more clean up to be done based on this, but I initially want to use this to represent the ABI details for the structs in Swift reverse pinvokes.

Comment threadsrc/coreclr/jit/abi.h
Comment on lines +12 to +13
bool IsPassedInRegister() const;
bool IsPassedOnStack() const;

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.

Is it relevant to track the other common qualifications like HFA and HVA?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I don't think so. HFA's/HVA's are just passed in more registers than other struct arguments, so hopefully by the end of this clean up that's a detail that's constrained fully to be within the ABI classification and not leaked out anywhere to the rest of the JIT.

Comment threadsrc/coreclr/jit/abi.h
Comment on lines +43 to +44
// - On arm64/arm32, HFAs can be passed in up to four registers, giving
// four register segments

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.

This should also include HVAs, which looks like we might be missing handling around:

godbolt HFA/HVA for Arm64

godbolt HVA/HVA for x64

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

These are just examples to indicate examples of the representation. The intention of this PR is not to make any functional changes.

@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.

Seems like a good start...

@jakobbotsch
jakobbotsch merged commit 13a9036 into dotnet:mainMar 25, 2024
@jakobbotsch
jakobbotsch deleted the refactor-abi branch March 25, 2024 17:34
jakobbotsch added a commit that referenced this pull request Apr 5, 2024
This adds the final support for frozen structs in UCO methods.
This passes 2500 auto generated tests locally on both macOS x64 and arm64. This
PR includes only 100 tests.
Frozen struct parameters are handled via the new ABI representation added in
#100138. When such a parameter exists we always allocate space for it on the
local stack frame. The struct is then reassembled from its passed constituents
as the first thing in the codegen.
One complication is that there can be an arbitrary amount of codegen to handle
this reassembling. We cannot handle an arbitrary amount of codegen in the
prolog, so the reassembling is handled in two places. First, since the amount of
register passed data is limited, we handle those in the prolog (which frees them
up to be used for other things). If some pieces were passed on the stack the JIT
then ensures that there is a scratch BB and generates the code to reassemble the
remaining parts as the first thing in the scratch BB.
Since Swift structs can be passed by reference in certain cases this PR also
enables `FEATURE_IMPLICIT_BYREFS` for SysV x64 to handle those cases. Depending
on the TP impact we can refine some of the ifdefs around this.
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 25, 2024
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 SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jakobbotsch@AndyAyersMS@tannergooding
, '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

JIT: Add a uniform representation for parameter ABI information - #100138

Merged
jakobbotsch merged 9 commits into
dotnet:mainfrom
jakobbotsch:refactor-abi
Mar 25, 2024
Merged

JIT: Add a uniform representation for parameter ABI information#100138
jakobbotsch merged 9 commits into
dotnet:mainfrom
jakobbotsch:refactor-abi

Conversation

@jakobbotsch

Copy link
Copy Markdown
Member

This adds a uniform representation that can represent the ABI
information for all of our targets without needing to fall back to
handling ABI specific details in all places that need to handle calling
conventions.

Currently nothing is using this information. I want to incrementally
migrate our ABI handling to use this representation. Also, there are
several potential future improvements:

  • Split out ABI classification per ABI instead of keeping them all
    within the same function
  • Unify InitVarDscInfo::stackArgSize and InitVarDscInfo::argSize. I
    am unsure why the latter is needed
  • Remove LclVarDsc::GetArgReg(), LclVarDscInfo::GetOtherArgReg(),
    HFA related members
  • Reuse the representation in CallArgABIInformation and unify the
    classifiers

The end goal here is rewriting genFnPrologCalleeRegArgs to handle
float and integer registers simultaneously, and to support some of the
registers that the Swift calling convention is using.

This adds a uniform representation that can represent the ABI
information for all of our targets without needing to fall back to
handling ABI specific details in all places that need to handle calling
conventions.
Currently nothing is using this information. I want to incrementally
migrate our ABI handling to use this representation. Also, there are
several potential future improvements:
- Split out ABI classification per ABI instead of keeping them all
within the same function
- Unify `InitVarDscInfo::stackArgSize` and `InitVarDscInfo::argSize`. I
am unsure why the latter is needed
- Remove `LclVarDsc::GetArgReg()`, `LclVarDscInfo::GetOtherArgReg()`,
HFA related members
- Reuse the representation in `CallArgABIInformation` and unify the
classifiers
The end goal here is rewriting `genFnPrologCalleeRegArgs` to handle
float and integer registers simultaneously, and to support some of the
registers that the Swift calling convention is using.
@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 22, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@jakobbotsch
jakobbotsch marked this pull request as ready for review March 25, 2024 10:35
@jakobbotsch

Copy link
Copy Markdown
MemberAuthor

cc @dotnet/jit-contrib PTAL @AndyAyersMS

No diffs. Some minor TP regressions (the seemingly large MinOpts ones are in collections with < 10 MinOpts contexts).

There's a lot more clean up to be done based on this, but I initially want to use this to represent the ABI details for the structs in Swift reverse pinvokes.

Comment threadsrc/coreclr/jit/abi.h
Comment on lines +12 to +13
bool IsPassedInRegister() const;
bool IsPassedOnStack() const;

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.

Is it relevant to track the other common qualifications like HFA and HVA?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I don't think so. HFA's/HVA's are just passed in more registers than other struct arguments, so hopefully by the end of this clean up that's a detail that's constrained fully to be within the ABI classification and not leaked out anywhere to the rest of the JIT.

Comment threadsrc/coreclr/jit/abi.h
Comment on lines +43 to +44
// - On arm64/arm32, HFAs can be passed in up to four registers, giving
// four register segments

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.

This should also include HVAs, which looks like we might be missing handling around:

godbolt HFA/HVA for Arm64

godbolt HVA/HVA for x64

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

These are just examples to indicate examples of the representation. The intention of this PR is not to make any functional changes.

@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.

Seems like a good start...

@jakobbotsch
jakobbotsch merged commit 13a9036 into dotnet:mainMar 25, 2024
@jakobbotsch
jakobbotsch deleted the refactor-abi branch March 25, 2024 17:34
jakobbotsch added a commit that referenced this pull request Apr 5, 2024
This adds the final support for frozen structs in UCO methods.
This passes 2500 auto generated tests locally on both macOS x64 and arm64. This
PR includes only 100 tests.
Frozen struct parameters are handled via the new ABI representation added in
#100138. When such a parameter exists we always allocate space for it on the
local stack frame. The struct is then reassembled from its passed constituents
as the first thing in the codegen.
One complication is that there can be an arbitrary amount of codegen to handle
this reassembling. We cannot handle an arbitrary amount of codegen in the
prolog, so the reassembling is handled in two places. First, since the amount of
register passed data is limited, we handle those in the prolog (which frees them
up to be used for other things). If some pieces were passed on the stack the JIT
then ensures that there is a scratch BB and generates the code to reassemble the
remaining parts as the first thing in the scratch BB.
Since Swift structs can be passed by reference in certain cases this PR also
enables `FEATURE_IMPLICIT_BYREFS` for SysV x64 to handle those cases. Depending
on the TP impact we can refine some of the ifdefs around this.
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 25, 2024
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 SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jakobbotsch@AndyAyersMS@tannergooding