microsoft / qsharp-language

Official repository for design of the quantum programming language Q# and its core libraries
MIT License
235 stars 56 forks source link

QIR: returning %Range type by value is problematic for native integration #31

Closed IrinaYatsenko closed 3 years ago

IrinaYatsenko commented 3 years ago

While returning composite types by value (i.e. ret %Range %r) seems to be valid LLVM's IR, Clang doesn't generate correct assembly code from it and returned value isn't placed on the stack where the caller expects to find it. For C++ functions that return custom types by value, Clang generates IR with an output parameter:

struct Foo { long long a; long long b; long long c; };
Foo ReturnByValue(long long x) { return {x, 3, 7}; }
define void @ReturnByValue(%struct.Foo* noalias sret %0, i64 %1) {
  ...
  ret void
} 

However, the spec requires to always pass and return %Range by value.

alan-geller commented 3 years ago

Hmm... My inclination would be, if we change the spec to return %Results by reference, we should also pass them by reference, and effectively change them from a value type to a reference type like %Result. I had originally been reluctant to do this because it seems like a lot of overhead for the common case where you create a %Range, iterate through it immediately, and then never use it again. I think, though, that we can detect that case and optimize away the heap allocation. @bettinaheim , @irinayat-MS , what do you think?

IrinaYatsenko commented 3 years ago

I agree, that having to allocate ranges on the heap is far from ideal. In my example, Clang doesn't put the struct on the heap but it still generates the function signature in terms of by-ref return.

alan-geller commented 3 years ago

One problem is that returning a %Result by ref implies allocating it on the heap, because it's not valid to allocate it anywhere else and return it by reference, unless the caller allocates it.

Having the caller allocate it would work, I suppose, since then you could use stack allocation. It would make the Q#->QIR compilation a bit more awkward, since anytime you call a routine you'd have to look at the return type to see what to do, and of course the same for when you're generating a new routine. Let me think about this a bit before jumping to a resolution; I'm also extremely open to any ideas you or anyone else might have!!

bettinaheim commented 3 years ago

Ultimately, this seems to be an issue only for interop and not for code within QIR itself. We hence went with keeping the specs as is, and put the burden on the interop handling to pass any ranges by pointer. See also https://github.com/microsoft/qsharp-compiler/issues/801 and https://github.com/microsoft/qsharp-compiler/pull/889.