[libswift] Refactor libSwift to use SIL types directly and remove bridged versions. #39958
Conversation
zoecarver
requested review from
gottesmm,
hyp,
guitard0g,
eeckstein and
ravikandhadai
| @@ -13,6 +13,10 @@ | |||
| import OptimizerBridging | |||
| import SIL | |||
|
|
|||
| extension Instruction { | |||
There was a problem hiding this comment.
This is a super weird hack that I'm trying to debug right now. I'm not sure if this has to do with extensions that use foreign types or what...
|
nice! |
zoecarver
force-pushed
the
pick-remove-silfunction-bridging
branch
from
cf97d7c to
64c92d5
Compare
|
@eeckstein @hyp @ravikandhadai and anyone else who might be interested: I just totally overhauled this PR. I updated it to remove both the C-bridging types and the Swift bridging types. We now simply extend the imported foreign reference types, and as you can see, the API doesn't change much (once we model inheritance correctly, it shouldn't change at all). I know there's a lot of code here, but I'd be interested in your overall thoughts. (Also, I know there are still some rough edges, this commit has been through a lot, and I'm going to spend some time cleaning it up next week.) |
| let context = PassContext(passContext: bridgedCtxt.passContext) | ||
| runFunction(function, context) | ||
| } | ||
| } | ||
|
|
||
| struct InstructionPass<InstType: Instruction> { |
There was a problem hiding this comment.
Woops, this is one of the rough edges: I can revert this change. Generics are working fine (as you can see with List).
There was a problem hiding this comment.
It's nice that you can get rid of the C bridging types and directly use the C++ types.
But the Swift API changes are not acceptable.
I'd like to emphasize once again: the swift SIL API should look like as if SIL was implemented in pure swift from scratch.
The most important goal is simplicity and not to make any compromises to make the clang importer happy.
I really think the way to go here is to keep the swift SIL classes and their var bridged computed member. Just let var bridged return the C++ classes instead of the C bridging type. As we discussed in a private chat some time ago.
| @@ -11,14 +11,15 @@ | |||
| //===----------------------------------------------------------------------===// | |||
|
|
|||
| import SIL | |||
| import SILBridging | |||
There was a problem hiding this comment.
It should not be required to import SILBriding in an optimization pass. SILBridging is an implementation detail of SIL
| if let tei = cfi.operand as? TupleExtractInst { | ||
| return tei.operand is BuiltinInst | ||
| private func hasOverflowConditionOperand(_ cfi: swift.CondFailInst) -> Bool { | ||
| if let tei = getAsTupleExtractInst(cfi.operand) { |
There was a problem hiding this comment.
No! That's a terrible API. Casts should be done with swift's native as? operator
| // Merge cond_fail instructions if there is no side-effect or read in | ||
| // between them. | ||
| for block in function.blocks { | ||
| // Per basic block list of cond_fails to merge. | ||
| var condFailToMerge = StackList<CondFailInst>(context) | ||
| var condFailToMerge = StackList<swift.CondFailInst>(context) |
There was a problem hiding this comment.
It should be CondFailInst, not any clang imported type from the SIL module
Sorry, Erik, I was meaning to discuss with out before posting this patch. I also should have elaborated on "the API doesn't change much." My reasoning here is that we don't really need the Swift classes, because we can have exactly the same API by just using extensions. Right now there are only two changes to how this API is consumed: sometimes we say |
That's not true. The swift classes are needed for casting.
No, we will not change the API temporarily. We can reconsider when the clang importer can do this (though, I see other problems, too). |
Why? If we bridge |
Specifically, this replaces BridgedInstruction, BridgedFunction, BridgedBasicBlock, and all optional variants to use SILFunction and Optional (etc.) directly.
This patch is based on #39605.
Refs #40293.