[pred-deadalloc-elim] Teach the pass how to eliminate dead allocation… #25167
Conversation
|
@swift-ci smoke test |
gottesmm
force-pushed
the
pr-bf6b4139d2234e0dc2ebfe47219f99758e8174ef
branch
from
8bd35f2 to
ce39822
Compare
|
@swift-ci smoke test |
| } | ||
|
|
||
| // CHECK-LABEL: sil [ossa] @diamond_test_4_with_load_take : $@convention(thin) (@owned Builtin.NativeObject, @owned NativeObjectPair) -> () { | ||
| // CHECK-NOT: alloc_stackfofofofofo |
There was a problem hiding this comment.
I'm pretty sure the compiler won't generate 'fofofofofofo' under any circumstance. Unless it has fear of falling over from owls flying overhead fast outside friendly offices.
| case PMOUseKind::Load: | ||
| // For now only handle takes from alloc_stack. | ||
| // | ||
| // TODO: It should be implementable, but it has not been needed yet. |
There was a problem hiding this comment.
How is Box vs. Stack relevant to this optimization? If it's all the same, don't distinguish them.
There was a problem hiding this comment.
It shouldn't. I just didn't need it and was avoiding writing some more tests. You are right though it is the /right/ thing to do.
…s that are load [take], store [init] back into memory. I am doing this to eliminate some differences in codegen before/after serialization ownership. It just means less of the tests need to be touched when I flip the switch. Specifically, today this change allows us to handle certain cases where there is a dead allocation being used to pass around a value at +1 by performing a load [take] and then storing a value back into the memory. The general format is an allocation that only has stores, load [take], and destroy_addr users. Consider the following SIL: ``` store %x to [init] %mem (0) %xhat = load [take] %mem (1) %xhat_cast = apply %f(%xhat) (2) store %xhat_cast to [init] %mem (3) destroy_addr %mem ``` Notice how assuming that we can get rid of the store, we can perform the following store -> load forwarding: ``` %xhat_cast = apply %f(%x) (2) store %xhat_cast to [init] %mem (3) destroy_addr %mem ``` In contrast, notice how we get an ownership violation (double consume of %x by (0) and (2)) if we can not get rid of the store: ``` store %x to [init] %mem %xhat_cast = apply %f(%x) store %xhat_cast to [init] %mem (2) destroy_addr %mem ``` This is in fact the same condition for promoting a destroy_addr since when a destroy_addr is a load [take] + destroy_value. So I was able to generalize the code for destroy_addr to handle this case.
gottesmm
force-pushed
the
pr-bf6b4139d2234e0dc2ebfe47219f99758e8174ef
branch
from
ce39822 to
ada2f9c
Compare
|
So I looked at the alloc_box issue and turns out what is actually happening is that we do not handle the destroy_value for alloc_box in general. I just didn't remember so I was being conservative. The pass never eliminates alloc_boxes. We could teach it though if we taught it how to understand that a destroy_value of the box is equivalent to a destroy_addr of a stack location. Since the pass has never supported this, I don't think that is within the scope of this PR. |
|
@swift-ci smoke test |
…s that are load [take], store [init] back into memory.
I am doing this to eliminate some differences in codegen before/after
serialization ownership. It just means less of the tests need to be touched when
I flip the switch.
Specifically, today this change allows us to handle certain cases where there is
a dead allocation being used to pass around a value at +1 by performing a load
[take] and then storing a value back into the memory. The general format is an
allocation that only has stores, load [take], and destroy_addr users. Consider
the following SIL:
Notice how assuming that we can get rid of the store, we can perform the
following store -> load forwarding:
In contrast, notice how we get an ownership violation (double consume of %x by
(0) and (2)) if we can not get rid of the store:
This is in fact the same condition for promoting a destroy_addr since when a
destroy_addr is a load [take] + destroy_value. So I was able to generalize the
code for destroy_addr to handle this case.