- Notifications
You must be signed in to change notification settings - Fork10.6k
[SIL][Optimizer] eliminate @pack_element types#85333
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 ourterms of service andprivacy 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
* SwiftCompilerSources/Sources/AST/Type.swift* SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyAllocStack.swift* SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyUncheckedAddrCast.swift* SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyWitnessMethod.swift* SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift* include/swift/AST/ASTBridging.h* include/swift/AST/ASTBridgingImpl.h
| return rawType.bridged.isSILPackElementAddress() | ||
| } | ||
| publicfunc getPackElementType(_ index:Int)->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.
It's better to add an API which returns all elements as an array, similar tovar packElements for SILPackType inSIL.Type
| ifoptimizeEnum(context){ | ||
| return | ||
| } | ||
| _=optimizeExistential(context) |
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.
If you addoptimizePackElement you need to return ifoptimizeExistential returns true, because that means that the alloc_stack is already deleted.
| /// Replaces an alloc_stack of an Pack Element by an alloc_stack of the concrete type. | ||
| /// | ||
| /// For example: | ||
| /// ``` |
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 you add thedynamic_pack_index andopen_pack_element instructions in this example to illustrate why this transformation can be done?
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.
Yes, of course.
| isLexical: isLexical, | ||
| isFromVarDecl: isFromVarDecl, | ||
| usesMoveableValueDebugInfo: usesMoveableValueDebugInfo) | ||
| forusein uses{ |
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.
There is a shorter way to replace all uses:uses.replaceAll(with:).
And in combination with erasing the original instruction:SingleValueInstruction.replace(with:)
| isFromVarDecl: isFromVarDecl, | ||
| usesMoveableValueDebugInfo: usesMoveableValueDebugInfo) | ||
| forusein uses{ | ||
| // FIXME: Are there any specific cases to handle? |
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.
yes! You cannot replace a value with a new value which has a different type without looking at the specific using instructions.
The safest thing to do is to cast the new alloc_stack back to the original pack existential type and let other peephole optimizations clean that up:
%1 = alloc_stack $@pack_element(...)->
%0 = alloc_stack $T %1 = unchecked_addr_cast %0 to $*@pack_element(...)For example, if an original use is anunchecked_addr_cast itself, the resulting double address casts will be optimized away.
However, you might need to add some peephole optimizations for the address cast, e.g. optimizeunchecked_addr_cast -witness_method pairs
| // ``` | ||
| // %1 = vector_base_addr %0 : $*Builtin.FixedArray<N, Element> | ||
| // ``` | ||
| _=optimizeVectorBaseCast(context) |
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.
Again, you need to return if this returns true
| // ``` | ||
| _=optimizeVectorBaseCast(context) | ||
| // ``` |
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.
Also here: please add the preceding pack instructions for clarity.
| letbuilder=Builder(before:self, context) | ||
| letnewCast= builder.createUncheckedAddrCast(from: fromAddress, | ||
| to:concreteType.loweredType(in: parentFunction).addressType) |
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.
wrong indentation
also, there should be space afterto:
| extensionWitnessMethodInst:Simplifiable,SILCombineSimplifiable{ | ||
| func simplify(_ context:SimplifyContext){ | ||
| _=tryReplaceExistentialArchetype(of:self, context) |
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.
again: return if it returns true
Most variadic generic functions contain loops that iterate over the elements of their Pack parameters. Since the types of the pack elements are unknown, witness table methods are used to perform operations on the current pack element in each iteration. After specialization, these loops are usually unrolled, since there is a fixed list of pack element types. In the unrolled code, it should be possible to eliminate the witness table lookups. This is not possible because of the @pack_element types introduced to refer to pack elements:
%5 = integer_literal $Builtin.Word, 0
%6 = dynamic_pack_index %5 of $Pack{Int, Double}
%7 = open_pack_element %6 of at <Pack{Int, Double}>, shape $each A, uuid "063BDCF4-98A3-11F0-B2E8-929C59BC796C"
%20 = witness_method $@pack_element("063BDCF4-98A3-11F0-B2E8-929C59BC796C") each A, #Numeric."*" …
%21 = apply %20<@pack_element("063BDCF4-98A3-11F0-B2E8-929C59BC796C") each A>(%10, %14, %18, %11) … // type-defs: %7
This simplification aims to eliminate all instances of @pack_element types from specialized functions by replacing those with the underlying concrete type. This allows the appropriate witness method, as well as other functions, to be inlined.
This computed attribute is used in the simplifications ofwitness_method,alloc_stack,unchecked_addr_cast andapply.