-
Notifications
You must be signed in to change notification settings - Fork 10.6k
LargeTypesReg2Mem: Avoid building an explosion schema just to count the number of registers for very large types #86148
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
base: main
Are you sure you want to change the base?
Conversation
|
@swift-ci test |
| /// detrimential i.e should be kept indirect. | ||
| enum IsVeryLargeType_t : bool { | ||
| IsNotVeryLargeType = false, | ||
| IsVeryLargeType = true |
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.
Maybe you can use the TeX convention: large Large LARGE huge Huge :)
lib/IRGen/LoadableByAddress.cpp
Outdated
| BuiltinFixedArrayType::MaximumLoadableSize; | ||
| entry.setNumRegisters(veryLargeTypeRegisterCount); | ||
| largeTypeProperties[ty] = entry; | ||
| return veryLargeTypeRegisterCount; |
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 find most of the code here unnecessarily confusing — I don't know why we're using a bunch of magic values instead of a well-encapsulated type. But this code seems to just be using the wrong magic value — I believe the magic value that needs to be stored in entry is MaxNumRegisters, which you can set by just calling setVeryLargeType().
...also, I know this is pre-existing code, but do you know why MaxNumRegisters is a mutable global variable?
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.
Fixed.
We compute properties (struct Properties) to make a determination whether SIL should be register to memory converted. Among the properties is the number of registers we deem a type to use (Properties::numRegisters). This field is a uint16_t and MaxNumRegisters is used to clamp when we set the field.
We also use this value when we want to say that an SIL type is very large.
When we make the decision whether to reg2mem promote the SIL the heuristic (LargeLoadableHeuristic) looks at the properties we computed. If the number of registers is bigger than some cutoff (15 currently LargeLoadableHeuristic::NumRegistersVeryLargeType) we immediately decide to reg2mem. If the number of registers is in-between 8 and 16 (LargeLoadableHeuristic::NumRegistersLargeType) we look at other properties such as number of uses.
…he number of registers for very large types
Computing the explosion schema for large types can be expensive.
E.g the following example would spend multiple seconds to compute the
explosion scheme for the array type. It is not worth adding the
complexity to have a special case explosion schema that handles types
like that since exploding a value like that is going to be detrimental.
```
struct Test {
private var values: [0xfffffff of [UInt32]] =
InlineArray(repeating: [])
}
```
rdar://165202495
6810a46 to
6c93043
Compare
|
@swift-ci test |
Computing the explosion schema for large types can be expensive.
E.g the following example would spend multiple seconds to compute the explosion scheme for the array type. It is not worth adding the complexity to have a special case explosion schema that handles types like that since exploding a value like that is going to be detrimental.
rdar://165202495