[sil] Add some small docs for copy_value, destroy_value, end_borrow, load_borrow #5453
Conversation
…load_borrow. rdar://28851920
|
@swift-ci Please smoke test and merge |
| end_borrow %1 from %0 : $T, $T | ||
| end_borrow %1 from %0 : $T, $*T | ||
| end_borrow %1 from %0 : $*T, $T | ||
| end_borrow %1 from %0 : $*T, $*T |
There was a problem hiding this comment.
If both operands are addresses, do we require them to be the same address?
There was a problem hiding this comment.
The idea is one could do something like this:
%1 = alloc_stack $T
borrow_addr %1 from %0 : $*T
...
end_borrow %1 from %0 : $*T, $*T
There was a problem hiding this comment.
Keep in mind, that I think we would only do this if we wanted to ensure that all borrowed addresses did not alias. Beyond that I can not see a good reason to have a "borrow" copy like operation that moves the borrowed value to a new memory location (but perhaps I am missing something).
I wrote this instruction purposely as generally as possible so that we could restrict it over time as we get a better idea of the exact constraints that we want here.
| // $T must be a loadable type | ||
|
|
||
| Loads the value ``%1`` from the memory location ``%0``. The ``load_borrow`` | ||
| instruction creates a borrowed scope in which a read-only borrow value ``%1`` |
There was a problem hiding this comment.
You should define what's a read-only borrow value is. I assume it's the same as for end_borrow (except %1 instead of %0):
- If
%1is an address,%1can not be written to. - If
%1is a non-trivial value,%1can not be destroyed.
There was a problem hiding this comment.
Actually, we can't load an address so 1. is not an option.
Somehow I'm missing information what a load_borrow actually "does". For example:
For non-trivial types it loads the value without retaining the value.
For trivial types... well, can we use it for trivial types?
There was a problem hiding this comment.
I think if I elaborate specifically on what this instruction does for the various categories of instructions (as you requested below), your question will be answered
| instruction creates a borrowed scope in which a read-only borrow value ``%1`` | ||
| can be used to read the value stored in ``%0``. The end of scope is deliminated | ||
| by an ``end_borrow`` instruction. All ``load_borrow`` instructions must be | ||
| paired with exactly one ``end_borrow`` instruction along any path through the |
There was a problem hiding this comment.
Is the "exactly one" really necessary? I have the feeling that it restricts the optimizer too much. Can't we have multiple end_borrow for a single load_borrow, like we do for alloc_stack-dealloc_stack?
Here is an example of a borrow-scope in a loop:
loop_head:
%1 = load_borrow %0
cond_br %c, loop_exit, loop_body
loop_body:
// some code
end_borrow %1 from %0
br loop_head
loop_exit:
end_borrow %1 from %0
There was a problem hiding this comment.
Notice how I said exactly one along any path through the program. That is the same condition as alloc-stack/dealloc-stack.
There was a problem hiding this comment.
You are right, I missed that
| ``%0``. Must be paired with at most 1 borrowing instruction (like | ||
| ``load_borrow``) along any path through the program. In the region in between | ||
| the borrow instruction and the ``end_borrow``, the original SILValue can not be | ||
| modified. This means that: |
There was a problem hiding this comment.
What requirements do we have for the end_borrow operands?
I assume that %0 and %1 must be the same "value" as %0 and %1 of load_borrow. Does it need to be the same SILValue? Or can it be a derived value? Example 1:
%1 = load_borrow %0 : $*T
%2 = upcast %1 to $Base // OK?
end_borrow %2 from %0
Example 2:
%1 = load_borrow %0 : $*T
%2 = struct_extract %1, #only_field_with_ref_semantics
%3 = struct_element_addr %0, #only_field_with_ref_semantics
end_borrow %2 from %3
There was a problem hiding this comment.
I imagine we will have some form of forwarding. Since borrows are in the type system, it should all just work.
| the borrow instruction and the ``end_borrow``, the original SILValue can not be | ||
| modified. This means that: | ||
|
|
||
| 1. If ``%0`` is an address, ``%0`` can not be written to. |
There was a problem hiding this comment.
Currently we only have load_borrow to begin a borrowing scope, right?
In this case %0 is always the address type of %1. Or am I missing something?
There was a problem hiding this comment.
No you are not missing anything. I was implementing a very general instruction that could be used for any of the borrowing cases that are interesting to us.
| 1. If ``%0`` is an address, ``%0`` can not be written to. | ||
| 2. If ``%0`` is a non-trivial value, ``%0`` can not be destroyed. | ||
|
|
||
| We require that ``%1`` and ``%0`` have the same type ignoring SILValueCategory. |
There was a problem hiding this comment.
You shouldn't use the term SILValueCategory, it's an implementation class. Maybe you can use "formal type" instead?
Again, isn't %0 always the address type of the object type %1?
There was a problem hiding this comment.
You could also have a borrow of an object from an object for instance. Or potentially an address type from an address type. Or even potentially an address from a value (for instance in the case of a re-abstraction thunk).
| ``unowned_retain`` and returning the operand. | ||
| 4. For aggregate types, this is equivalent to recursively performing a | ||
| ``copy_value`` on its components, forming a new aggregate from the copied | ||
| components, and then returning the new aggregate. |
There was a problem hiding this comment.
This list is a very good description. It would help if you could write something similar for load_borrow and end_borrow.
There was a problem hiding this comment.
Yes. I think doing such a thing will provide the clarity that you want above.
[pull] swiftwasm from main
[sil] Add some small docs for copy_value, destroy_value, end_borrow, load_borrow
rdar://28851920