[SILCombine] Dominance check for inject_enum_addr. #73040
Conversation
|
@swift-ci please test |
| @@ -1219,7 +1246,7 @@ SILCombiner::visitInjectEnumAddrInst(InjectEnumAddrInst *IEAI) { | |||
| // store %1 to %nopayload_addr | |||
| // | |||
| auto *AI = dyn_cast_or_null<ApplyInst>(getSingleNonDebugUser(DataAddrInst)); | |||
| if (!AI) | |||
| if (!AI || !DI->properlyDominates(AI, IEAI)) | |||
There was a problem hiding this comment.
Should the dominance check be limited to empty types ? For non empty types we do not have uninitialized paths and won't need this check? With the check we may not be optimizing the case where one path could have the initializing apply, and another uninitialized path may end in an unreachable.
There was a problem hiding this comment.
With the check we may not be optimizing the case where one path could have the initializing apply, and another uninitialized path may end in an unreachable.
It seems like, even if the address is uninitialized on some unreachable-terminated path, every path through IEAI would still go through AI``* (AI would dominate IEAI). But...
For non empty types we do not have uninitialized paths and won't need this check?
Great point! This check is wasteful for non-empty types. Added a check that the type is empty before checking dominance. And added some test cases with dead end blocks after the init_enum_data_addr.
* Or some other initialization in which case there's another bailout since the apply case only handles the case where the apply is the single use.
| auto pair = pairs[0]; | ||
| assert(pair.has_value()); | ||
| if (!DI->properlyDominates(pair->second, forInst)) | ||
| return std::nullopt; |
There was a problem hiding this comment.
Looks like without the dominance check here, we can have bad SIL with non empty types too!?
There was a problem hiding this comment.
Good point, we can't have them here either because that would imply there was a memory location whose type is non-empty which wasn't initialized on some path, which is invalid SIL.
nate-chandler
force-pushed
the
rdar126231554
branch
from
1366f73 to
9661193
Compare
nate-chandler
force-pushed
the
rdar126231554
branch
from
9661193 to
e4099da
Compare
If stores of empty values to the address don't dominate the inject_enum_addr, the inject_enum_addr can't be eliminated--it's the only indication of the case that's in the address.
nate-chandler
force-pushed
the
rdar126231554
branch
from
e4099da to
3070f83
Compare
|
@swift-ci please test |
If stores of empty values to the address don't dominate the
inject_enum_addr, theinject_enum_addrcan't be eliminated--it's the only indication of the case that's in the address.