-
Notifications
You must be signed in to change notification settings - Fork 10
[CHERIoT] Pass two-caps return values and arguments in registers #178
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
[CHERIoT] Pass two-caps return values and arguments in registers #178
Conversation
9e0b1cf to
c297697
Compare
c297697 to
c6163f7
Compare
d2d2889 to
f7259fb
Compare
clang/lib/CodeGen/Targets/RISCV.cpp
Outdated
|
|
||
| if (auto *RT = Ty->getAs<RecordType>()) { | ||
| unsigned MaxCapRegs = IsCheriot ? 2 : 1; | ||
| return Size == MaxCapRegs * getTarget().getCHERICapabilityWidth() && |
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 this be <= rather than ==?
clang/lib/CodeGen/Targets/RISCV.cpp
Outdated
|
|
||
| bool IsCapability = Ty->isCHERICapabilityType(getContext()) || | ||
| IsSingleCapRecord; | ||
| (ForcePassInCapRegs && Size == CapabilityWidth); |
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.
This could be just ForcePassInCapRegs if you change == to <= in shouldPassStructDirectInCapRegisters
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.
Per chat discussion, if we can't simplify the logic, perhaps we can rename the variables to make the logic simpler?
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.
Here's the relevant bit of the discussion: the final goal is that of having IsCapability true iff. the type is itself a capability or a struct made of a single capability, the case where the struct is composed of >= 2 capabilities should not entail IsCapability == true.
Latest commit renames IsCapability to IsSingleCapability!
clang/lib/CodeGen/Targets/RISCV.cpp
Outdated
|
|
||
| bool IsCapability = Ty->isCHERICapabilityType(getContext()) || | ||
| IsSingleCapRecord; | ||
| (ForcePassInCapRegs && Size == CapabilityWidth); |
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.
Same as above
c890fbc to
0a08dd7
Compare
|
The previous code was not taking into account the case for |
0a08dd7 to
ab0982e
Compare
resistor
left a comment
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.
This is way better than before!
|
|
||
| /// CHERI(oT)-specific: Figure out how many registers are needed to pass a | ||
| /// value of the given type and size directly in register(s). The value of | ||
| /// `-1` means that the value should be passed indirectly. |
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.
Can't 0 be the sentinel?
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 set it to -1 because I am not completely sure about the semantics of zero-sized types and, perhaps, there might be a situation where we want to know that the value "fits in 0 registers", if that makes any sense.
Should close #177. This PR also adds the same mechanism for arguments that can be subjected to the same optimisation.