Relaxed the checks in SILDeserializer::readSILFunctionChecked()

[mhong]

to allow deserializing SIL function decls/signatures (but not bodies) during IRGen phase.

This is needed in the tensorflow branch, where we want to generate calls to swift
functions (via their silgen names) in IRGen. e.g. the call generated at
https://github.com/apple/swift/blob/ad7def2c6bffd95d62e5e665c9faed0f8dac49f5/lib/IRGen/IRGenSIL.cpp#L1956-L1957,
where the swift function is defined at https://github.com/apple/swift/blob/ad7def2c6bffd95d62e5e665c9faed0f8dac49f5/stdlib/public/TensorFlow/CompilerRuntime.swift#L1163-L1167.

#19524 has more context.

[slavapestov]

@shajrawi is my comment correct?

[slavapestov]

I'm not sure if this is correct, because the large loadable types pass changes function signatures as well.

[shajrawi]

It does change function signatures yes.

[lattner]

@rjmccall and @jckarter this patch is really a question: we need to synthesize calls to runtime functions in IRGen and would love to get the proper signature synthesized by the clang importer for these functions (we're just dealing with primitive C types here like floats, ints and pointers, nothing fancy) instead of putting them in RuntimeFunctions.def. Does this approach seem sensible?

[slavapestov]

@mhong Why do you need to generate the call in IRGen? Can you have a SIL pass that runs late in the pipeline do it instead?

[slavapestov]

@lattner I'm worried that even if this approach is correct for your use case, the relaxed assertion will be wrong in the general case and bugs might slip in later.

[lattner]

@slavapestov yes, it would be super easy for us to lower our constructs as a SIL -> SIL pass, but it would have to be a mandatory pass that runs after the performance optimizer passes. It seems cleaner conceptually to just handle this in IRGen, but if there is precedent here to do this sort of thing as a SIL to SIL pass (which one?) then we're super super happy to do that. We're trying to do the right but hard thing here, if there is an easy path, then we'd love guidance on that :)

[lattner]

see also PR19555. We have something like 20 APIs we'll need to generate calls into...

[slavapestov]

@lattner I think you'll only find imported functions by name in the SIL module if they were previously referenced from something emitted by SILGen. It doesn't sound like a foolproof approach. I think @jckarter or @DougGregor will have more actionable suggestions for you though. I'm just mostly concerned about the loosened assertion. And if you need this to make progress while you work on a more complete solution, I won't complain about you merging this.

[rjmccall]

We already widely assume that C functions just involving pointers and pointer-sized integers have a predictable lowering into IR. I don't know any reason why that wouldn't equally hold for floating-point types. I'd just use RuntimeFunctions.def.

[mhong]

Thanks all for your input! We'll go with the runtime function based approach, and abandon this patch.

A few follow-up question and notes with @slavapestov:

1. Regarding "the large loadable types pass changes function signatures", are there any pointers on the why and how? I feel this adds some internal complexity.

2. Regarding your note below:

I think you'll only find imported functions by name in the SIL module if they were previously referenced from something emitted by SILGen.

Indeed I've experienced that, and I found it's worse than that. Our TensorFlow swift module imports the CTensorFlow module (e.g. https://github.com/apple/swift/blob/ad7def2c6bffd95d62e5e665c9faed0f8dac49f5/stdlib/public/TensorFlow/CompilerRuntime.swift#L50), where the latter has a set of C APIs (https://github.com/apple/swift/blob/ad7def2c6bffd95d62e5e665c9faed0f8dac49f5/stdlib/public/CTensorFlow/module.modulemap#L2). The compiled TensorFlow module then contains some of the C APIs (e.g. TF_NewStatus()), but not others (e.g. TFE_NewOp()), even though TFE_NewOp() is referenced in CompilerRuntime.swift. This caused IRGen over a user module not able to find the TFE_NewOp() API (through silModule.findFunction()). I don't understand why.

A work-around is that in the user module code, we call TFE_NewOp(), so that compiler will see that code before entering the IRGen phase. This is not a good long term solution.

For that reason and some other factors (listed as "concerns" in the PR description of #19555), we are going with the runtime function approach instead of clang importer (i.e., silModule.findFunction()).