<div dir="ltr">On Wed, Dec 13, 2017 at 4:14 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span class=""><blockquote type="cite"><div>On Dec 13, 2017, at 6:42 PM, Saleem Abdulrasool <<a href="mailto:compnerd@compnerd.org" target="_blank">compnerd@compnerd.org</a>> wrote:</div><br class="m_4166797275285532235Apple-interchange-newline"><div><div style="word-wrap:break-word;line-break:after-white-space"><br><div><br><blockquote type="cite"><div>On Dec 13, 2017, at 12:46 PM, John McCall <<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>> wrote:</div><br class="m_4166797275285532235Apple-interchange-newline"><div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><blockquote type="cite"><div><br class="m_4166797275285532235Apple-interchange-newline">On Dec 13, 2017, at 3:22 PM, Saleem Abdulrasool <<a href="mailto:compnerd@compnerd.org" target="_blank">compnerd@compnerd.org</a>> wrote:</div><br class="m_4166797275285532235Apple-interchange-newline"><div><div style="word-wrap:break-word;line-break:after-white-space"><br><div><br><blockquote type="cite"><div>On Dec 13, 2017, at 12:14 PM, John McCall <<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>> wrote:</div><br class="m_4166797275285532235Apple-interchange-newline"><div><blockquote type="cite" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><br class="m_4166797275285532235Apple-interchange-newline">On Dec 13, 2017, at 2:56 PM, Saleem Abdulrasool <<a href="mailto:compnerd@compnerd.org" target="_blank">compnerd@compnerd.org</a>> wrote:<br><br>Hey guys,<br><br>I have another fun case that things go wrong in :-). We do not preserve the calling convention information when importing function decls via the ClangImporter. I have a pretty simple example in apple/swift#13404. Importing the following:<br><br><blockquote type="cite">float frand(void);<br>float fadd(float f, float g) { return f + g; }<br></blockquote><br>and using this in swift as:<br><br><blockquote type="cite">func f -> Float { return fadd(frand(), frand()); }<br></blockquote><br><br>will result in the fadd call being elided due to the UB in the CC mismatch. Im pretty sure that we should be preserving CC information when importing the interface, probably in `VisitFunctionDecl` in ImportDecl.cpp (although, I believe it can also be lazily computed). Im not sure which really would be the best thing to do here.<br><br>This can show up on other targets when interfaces uses `__attribute__((__pcs__))` or `__attribute__((__fastcall__))<wbr>`, `__attribute__((__vectorcall__<wbr>))`, `__attribute__((__regparm__([<wbr>1-3])))`, `__attribute__((__stdcall__))`<wbr>, `__attribute__((__thiscall__))<wbr>`.<br></blockquote><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">Without enhancing SIL, I'm not sure we have a good option besides refusing to import function declarations with non-standard CCs.</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">I don't think we really want to change AST function types to handle arbitrary imported calling conventions, but we could change SILFunctionType to be able to store a Clang CC. However, this will require a little bit of extra work in that, if someone tries to pass around the address of a fastcall function as a @convention(c) function, we will have to introduce a thunk. I believe we already do similar kinds of thunking in SILGen, though.</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"></div></blockquote><div><br></div><div>This is slightly problematic as Linux ARM HF and Windows ARM both use a non-C default (arm_aapcs_vfpcc) which is the test case in the mentioned PR. That is, even without the attributes the declarations above have a non-C CC.</div><div><br></div><div>I don't think that we are supporting arbitrary calling conventions per se. This only becomes a problem at the FFI layer, where we want to convert the swift CC to the foreign CC. The IRGen at that point will generate UB which will truncate the implementation when the LLVM optimizer runs. I think that we should at least support AAPCS in both, the standard and VFP, variants at the very least.</div></div></div></div></blockquote><div><br></div></div><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">Supporting two C calling conventions is not easier than supporting arbitrary C calling conventions. All the complexity is in how and where we represent those CCs. I'm hesitant to introduce this complexity (and non-portability) into the core language, which is why I'm suggesting just introducing it into SIL. But I explicitly do not want us to hard-code specific non-standard conventions from C outside of maybe attribute parsing; we should generalize the representation to support arbitrary CCs. I don't think this is hard at the SIL level.</span></div></blockquote><div><br></div><div>Hmm, I think that we might be looking at different problems. I want to support the de facto standard CC on the target. If you consider Windows ARM, “c” is entirely invalid.</div></div></div></div></blockquote><div><br></div></span><div>@convention(c) cannot be "entirely invalid". @convention(c) means the platform's C calling convention, which I promise does always exist. The code "int foo(void); int main() { foo(); }" does actually compile on Windows ARM, and it uses a calling convention; that is the platform's C calling convention.</div></div></div></blockquote><div><br></div><div>Ah, I was interpreting it to be equivalent to LLVM's notion of the C calling convention which is not the same as you describe it. On Windows ARM, the C convention requires an additional annotation of the function call.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span class=""><blockquote type="cite"><div><div style="word-wrap:break-word;line-break:after-white-space"><div><div>The equivalent of “c” is arm_aapcs_vfpcc. However, there are other cases where we do need a secondary CC. As an example of that, the AEABI RT function calls are always made as AAPCS, but on Linux ARM HF targets (e.g. armv7-unknown-linux-gnueabihf)<wbr>, the default CC is the VFP variant of AAPCS, so we do need a secondary CC there for some library calls. We usually get lucky as the function calls there are formed by the backend which knows the CC necessary for the call there.</div></div></div></div></blockquote><div><br></div></span>I think you might be confusing "the C calling convention" with some other concept — maybe the default LLVM calling convention? I believe it's true that LLVM has backends, notably ARM, where the maintainers made the (IMO questionable) decision to have the default LLVM CC be the same across operating systems instead of considering the full target triple. That just means that frontends have an obligation to set the target's actual CC on every C call and function. I would readily believe that Swift IRGen has bugs — maybe even pervasive ones — where we fail to do that.</div></div></blockquote><div><br></div><div>Yes, this is the scenario that I was describing - the need for the frontend to annotate the call with he correct calling convention for the target (which is equivalent to swift's notion of `@convention(c)`). However, the CC annotation is entirely missing even for uses of the declarations as the information is not preserved during the import stage.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span class=""><blockquote type="cite"><div><div style="word-wrap:break-word;line-break:after-white-space"><div><blockquote type="cite"><div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">In general, the importer has a lot of flexibility when importing declarations and very little flexibility when importing types, especially pointer types. We can only import one kind of function pointer type as a @convention(c) function type; everything else needs to be imported as an opaque pointer unless we actually bite the bullet and enhance the AST @convention system to support more C conventions. For maximum expressivity, that function pointer type needs to be the type most commonly used for function pointers on the platform, assuming there is one. It doesn't really matter if that's not the default calling convention for function *declarations* because we can always pass around a specific variant-CC function as a @convention(c) function value by introducing a thunk; function-pointer equality won't work, but probably nobody cares. What we can't do is turn an arbitrary variant-CC function *pointer* into a @convention(c) function pointer. But the correctness of all this relies on SIL being able to fully represent the C calling convention.</div></div></blockquote><div><br></div><div>In the case of support of a target which does not support the C calling convention but expects all calls to be of a specific convention, we currently fall apart. It sounds like you would prefer that the approach to handle that would be to map `@convention(c)` to that alternate calling convention? For importing declarations, we currently do not preserve the calling convention at all. As a result, right now, FFI calls into C may introduce UB in the IR.</div></div></div></div></blockquote><div><br></div></span>Yes, we clearly have a responsibility to call things with the right calling convention, and we should not import types and declarations which we cannot meet that responsibility for. I am trying to describe the correct technical design for that, which is:</div></div></blockquote><div><br></div><div>Great, so we agree on this :-).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div> - In the importer, we do not need to assign a specific convention to the type of an imported function declaration.</div></div></blockquote><div><br></div><div>I don't understand how this works when we may need to annotate the function call to match the C calling convention.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div> - In the importer, we should import function pointer types as @convention(c) function types if they use the target's C calling convention and as opaque pointer types otherwise.</div></div></blockquote><div><br></div><div>Fair enough.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div> - SILFunctionType should have the ability to express a calling convention more precisely than just @convention(c). IRGen can just use this value when explicit. For @convention(c), IRGen can ask Clang for the correct LLVM CC to use.</div></div></blockquote><div><br></div><div>Agreed that the calls should be able to declare this more precisely. Do we want to round trip back to clang to get the LLVM CC for this call? Especially if this is the "C" CC?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div> - SIL type lowering should assign function declarations their actual C calling convention if they use a non-standard calling convention.</div></div></blockquote><div><br></div><div>Yes.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div> - SILGen may need to introduce thunks when passing around such functions as @convention(c) function values. Code for this already exists in order to allow C functions to be passed around as native function values.</div></div></blockquote><div><br></div><div>This sounds pretty good and would broaden the abilities for swift to FFI to existing code.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><span class="HOEnZb"><font color="#888888"><div>John.</div></font></span><span class=""><div><br></div><div><blockquote type="cite"><div><div style="word-wrap:break-word;line-break:after-white-space"><div><div>I think that the FFI to declarations is far more common than the FFI to a function pointer, which is why I am focusing on the imported declaration rather than the imported types. I do agree that supporting additional conventions in the SIL layer would be good for the imported function pointer case. Am I missing something and is the `@convention` used for the imported declarations too?</div><br><blockquote type="cite"><div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">John.</div></div></blockquote></div></div></div></blockquote></div></span></div></blockquote></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">Saleem Abdulrasool<br>compnerd (at) compnerd (dot) org</div>
</div></div>