[CastOptimizer] Set the correct debug scope for the SILBuilder. #25490
Conversation
<rdar://problem/48887384>
|
Fixes (another) verification failure. |
|
@swift-ci please test |
|
LGTM. We should rename SILBuilder to something else and make SILBuilderWithScope, SILBuilder. |
Perhaps, but it's difficult. @vedantk and I thought a lot about this in the past and identified at least three common and very different use-cases for SILBuilders, including generating new code from AST, expanding one SIL instruction into many, moving SIL instructions, inlining, ... all have different needs. Originally we thought that we could introduce an Expanding/Moving/GeneratingSILBuilder but we couldn't settle on a design that looked right. I agree that we should revisit that. |
|
@adrian-prantl I disagree with that strongly. We should fix this now if possible. It is creating bugs /today/. My feeling is that unless someone is in SILGen, they should be using SILBuilderWithScope. Why not take advantage of that to hide the original SILBuilder from the SILOptimizer. I already in a previous commit (that I just discovered Slava reverted for some reason, I pinged him about it) created a specific SILBuilder for SILGen. We could have a SILBuilderBase that has the common APIs in between GenSILBuilder and OptSILBuilder. Your thoughts? |
|
@adrian-prantl My memory was actually wrong, we still have the SILGenBuilder in SILGen. We could put the special SILGen APIs on that. |
|
As the person involved in fixing (way too many) of these bugs, my opinion is that delaying this refactoring is becoming an increasing burden, so I agree with @gottesmm here. |
|
This broke in CI: https://ci.swift.org/job/oss-swift_tools-RA_stdlib-DA_test-simulator/687/ rdar://problem/51804780 I'm going to revert: #25512 |
rdar://problem/48887384