[ownership] Add a subclass for all OwnershipForwarding instructions and fix the classof to the various ownership abstract classes so isa/dyn_cast work. #34809
Conversation
|
@swift-ci smoke test |
|
Looks like I broke something when testing locally in SemanticARCOpts... Another land mine. |
|
Figured it out. The problem was that we were relying on SILArgument being recognized as an owned forwarding value (that is the result of an instruction that forwards owned ownership) due to us erroneously calling br an owned forwarding inst. (Generally we want owned forwarding constructs to suggest a dominating relationship rather than the br which can introduce value phis). We were taking advantage of that check as well for transforming terminators, so we stopped eliminating copies that feed into checked_cast_br/switch_enum diamonds. |
…nd fix the classof to the various ownership abstract classes so isa/dyn_cast work. I think what was happening here was that we were using one of the superclass classofs and were getting lucky since in the place I was using this I was guaranteed to have single value instructions and that is what I wrote as my first case X ). I also added more robust checks tieing the older isGuaranteed...* APIs to the ForwardingOperand API. I also eliminated the notion of Branch being an owned forwarding instruction. We only used this in one place in the compiler (when finding owned value introducers), yet we treat a phi as an introducer, so we would never hit a branch in our search since we would stop at the phi argument. The bigger picture here is that this means that all "forwarding instructions" either forward ownership for everything or for everything but owned/unowned. And for those listening in, I did find one instruction that was from an ownership forwarding subclass but was not marked as forwarding: DifferentiableFunctionInst. With this change, we can no longer by mistake have such errors enter the code base.
gottesmm
force-pushed
the
pr-584924e29ff9f53ce445cbfbe808237ac89ac4a9
branch
from
42f540b to
c35ff33
Compare
|
@swift-ci smoke test |
|
@swift-ci smoke test linux platform |
|
I think I came up with a.way to do this without having a classof assert in every constructor call. |
I think what was happening here was that we were using one of the superclass
classofs and were getting lucky since in the place I was using this I was
guaranteed to have single value instructions and that is what I wrote as my
first case X ).
I also added more robust checks tieing the older isGuaranteed...* APIs to the
ForwardingOperand API. I also eliminated the notion of Branch being an owned
forwarding instruction. We only used this in one place in the compiler (when
finding owned value introducers), yet we treat a phi as an introducer, so we
would never hit a branch in our search since we would stop at the phi argument.
The bigger picture here is that this means that all "forwarding instructions"
either forward ownership for everything or for everything but owned/unowned.
And for those listening in, I did find one instruction that was from an
ownership forwarding subclass but was not marked as forwarding:
DifferentiableFunctionInst. With this change, we can no longer by mistake have
such errors enter the code base.