-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RISC-V][LoongArch64] New passing info for floating-point structs #103945
Merged
Merged
Changes from all commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
dcf2edb
Replace StructFloatFieldInfoFlags with FpStructInRegistersInfo which …
tomeksowi 48caa9c
Replace StructFloatFieldInfoFlags with FpStruct::Flags in profiler
tomeksowi 853bd99
Remove FpStructInRegistersInfo::FromOldFlags()
tomeksowi 0e80ccf
Fix duplicating types in HandleInlineArray
tomeksowi 3d139ac
Remove signedness from FpStruct::IntKind because most probably we won…
tomeksowi 89cc148
Remove old StructFloatFieldInfoFlags calculating routine
tomeksowi c2984a2
Typo in TARGET_LOONGARCH64
tomeksowi a229831
Remove m_returnedFpFieldOffsets from ArgIterator
tomeksowi f2b38f8
Add missing ENREGISTERED_PARAMTYPE_MAXSIZE condition to C# version of…
tomeksowi 5481693
Rename RISCV64PassStructInRegister to match settled casing for RiscV …
tomeksowi 4250bf4
Update hardcoded flags for float and double in ArgIteratorTemplate::C…
tomeksowi 0f03a88
Fix build on other platforms
tomeksowi f485362
Update LoongArch to use FpStructInRegistersInfo
tomeksowi 9967e2e
Remove unused old flag masks
tomeksowi 6bc28d1
LoongArch64 typo
tomeksowi 33a6aee
Missing FpStruct namespace
tomeksowi 1e951bd
Missing FpStruct namespace
tomeksowi fd49a25
Missing FpStruct namespace
tomeksowi 214b94f
Use FpStruct namespace everywhere in JIT
tomeksowi e34a68d
Merge branch 'main' into fp-struct-info
tomeksowi 08c6385
JIT review
tomeksowi f163a78
Merge branch 'main' into fp-struct-info
tomeksowi e23c0ab
Update StructFloatFieldInfoFlags description
tomeksowi ef5a6d4
Revert to hitherto instruction set order as it's not the point of thi…
tomeksowi 100eced
Unify get{LoongArch,RiscV}64PassFpStructInRegistersInfo JIT interfaces
tomeksowi 5c9e5ac
Use JIT_TO_EE_TRANSITION instead of _LEAF because MethodTable::GetFpS…
tomeksowi 1d5ad91
Remove FpStruct::IntKind, we should have similar info in ClassLayout …
tomeksowi 7b1587b
Change JIT interface to return a struct similar to CORINFO_SWIFT_LOWE…
tomeksowi e37e984
Change JIT to use new Swift-like getFpStructLowering
tomeksowi c7c88e0
Cache CORINFO_FPSTRUCT_LOWERING
tomeksowi 5c478d7
Update LoongArch classifier to use CORINFO_FPSTRUCT_LOWERING
tomeksowi 1f16783
Update StructFloatInfoFlags doc comment on C#
tomeksowi f064eb6
Move StructFloatFieldInfoFlags and FpStructInRegistersInfo out of the…
tomeksowi d4be92c
Merge LoongArch and RISC-V AOT calculation of FpStructInRegistersInfo…
tomeksowi 823df8b
Don't zero-initialize CORINFO_FPSTRUCT_LOWERING
tomeksowi c209a55
Merge branch 'main' into fp-struct-info
tomeksowi 8ddd349
Add note for CORINFO_FPSTRUCT_LOWERING::loweredElements type
tomeksowi 9dc5ed5
object is lowercase
tomeksowi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my edification, what's the calling convention of
struct { object o; float a; }
?Is it passed using this special lowering or using integer call conv?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to floating-point calling convention (special lowering). Object is a pointer, which for the convention is simply an integer field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the Object field from my example be reported as
CORINFO_TYPE_CLASS
in this struct then? It does not look right that it is reported as int.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we agreed that JIT doesn't need to know IntKind (it was used to tell ordinary integers from GC refs), because it can probe for GC refs from the layout it knows already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently JIT only needs to know from CORINFO_FPSTRUCT_LOWERING whether the type is floating or integer and the type size. GC info is taken from class layout.
I'd like to merge as is if it's not a problem, this PR blocks #104237 and subsequent changes stemming from #101796. If it turns out that the integer type needs to be more precisely specified for some reason, we can always amend it and FpStructInRegistersInfo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ping. Can this be merged?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add a comment that the types returned in
loweredElements
only represent the size and not the actual type?(An alternative would be to change
loweredElements
touint32_t loweredElementsSizes[]
to make it obvious that the API just returns the size and not the actual type.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added comment. The alternative would need another bool array for floating vs. integer and would stray from CORINFO_SWIFT_LOWERING if the need to handle ABI lowering in shared code ever comes up.