heavyai / rbc

Remote Backend Compiler
https://rbc.readthedocs.io
BSD 3-Clause "New" or "Revised" License
30 stars 10 forks source link

Weirdness in returning Buffer objects #224

Open pearu opened 3 years ago

pearu commented 3 years ago

A UDF returning Array or any other Buffer object must explicitly copy the data to the returning object. For instance,

    @omnisci('int32[](int32[])')
    def mypass(s):
        r = Array(len(s), 'int32')
        for i in range(len(s)):
            r[i] = s[i]
        return r

works correctly but the following examples fail with server crash:

    @omnisci('int32[](int32[])')
    def mypass(s):
        r = Array(len(s), 'int32')
        return r

    @omnisci('int32[](int32[])')
    def mypass(s):
        return s

The second example is related to #123

The third example produces the following LLVM IR:

define void @mypass__cpu_0({ i32*, i64, i8 }* nocapture sret %.1, { i32*, i64, i8 }* nocapture readonly byval({ i32*, i64, i8 }) %.2) local_unnamed_addr #6 {
entry:
  %0 = bitcast { i32*, i64, i8 }* %.1 to i8*
  %1 = bitcast { i32*, i64, i8 }* %.2 to i8*
  tail call void @llvm.memmove.p0i8.p0i8.i64(i8* nonnull align 8 dereferenceable(24) %0, i8* nonnull align 8 dereferenceable(24) %1, i64 24, i1 false)
  ret void
}

which shows that the input Buffer struct is returned as shallow copy (pointer values are copied) but the copy ought to be deep copy (allocate new memory and copy data pointed by the pointer value).

Possible actions:

guilhermeleobas commented 3 years ago

We can use as_return method to copy data from the %2 to %1 in the case below

define void @mypass__cpu_0({ i32*, i64, i8 }* nocapture sret %.1, { i32*, i64, i8 }* nocapture readonly byval({ i32*, i64, i8 }) %.2) local_unnamed_addr #6 {
entry:
  %0 = bitcast { i32*, i64, i8 }* %.1 to i8*
  %1 = bitcast { i32*, i64, i8 }* %.2 to i8*
  tail call void @llvm.memmove.p0i8.p0i8.i64(i8* nonnull align 8 dereferenceable(24) %0, i8* nonnull align 8 dereferenceable(24) %1, i64 24, i1 false)
  ret void
}

Edit: I guess if we replace the memmove by a memcopy operation, this problem will be solved.

pearu commented 3 years ago

This issue has likely the same origin as #77