[region-isolation]fix crash due to missing VectorBaseAddrInst case #81591
Conversation
|
@swift-ci smoke test |
| @@ -312,6 +312,7 @@ static bool isStaticallyLookThroughInst(SILInstruction *inst) { | |||
| case SILInstructionKind::UncheckedEnumDataInst: | |||
| case SILInstructionKind::StructElementAddrInst: | |||
| case SILInstructionKind::TupleElementAddrInst: | |||
| case SILInstructionKind::VectorBaseAddrInst: | |||
There was a problem hiding this comment.
I think that you need to add a visitor case below.
There was a problem hiding this comment.
I see it was already added below. The best way to make sure this does not happen in the future is for people to add a test in https://github.com/swiftlang/swift/blob/main/test/Concurrency/regionanalysis_trackable_value.sil. Can you add a test case for this there.
There was a problem hiding this comment.
Thank you for the advice.
I have added a test based on the debug log, but it does not reproduce the issue.
Could you suggest what kind of test I should add?
| @@ -312,6 +312,7 @@ static bool isStaticallyLookThroughInst(SILInstruction *inst) { | |||
| case SILInstructionKind::UncheckedEnumDataInst: | |||
| case SILInstructionKind::StructElementAddrInst: | |||
| case SILInstructionKind::TupleElementAddrInst: | |||
| case SILInstructionKind::VectorBaseAddrInst: | |||
There was a problem hiding this comment.
I see it was already added below. The best way to make sure this does not happen in the future is for people to add a test in https://github.com/swiftlang/swift/blob/main/test/Concurrency/regionanalysis_trackable_value.sil. Can you add a test case for this there.
|
@stzn Can you rebase. I want to land this ASAP. This got reported to me in another place and I want to get this into 6.2. |
|
Also, the test was done incorrectly. You want to do a debug_value [trace] on %2. |
2e5d758 to
b4b8159
Compare
|
Thank you. |
b4b8159 to
c9e70b7
Compare
|
@stzn the test shows that something is missing in your implementation. We should look through the vector_base_addr if it is look through. |
Resolves #81534
It seems we are missing the case for
VectorBaseAddrInst.VectorBaseAddrInstis statically defined asLookThrough.swift/lib/SILOptimizer/Analysis/RegionAnalysis.cpp
Line 3414 in d985bf6
Regarding tests, similar problems could arise when new
LookThroughinstructions are added in the future. To prevent this, we should consider finding a way to ensure allLookThroughinstructions are properly handled inisStaticallyLookThroughInst. What do you think? Are there any other places that do something similar?