<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">Hi Ken,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Feb 6, 2019 at 4:09 PM <<a href="mailto:commits@source.squeak.org">commits@source.squeak.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> <br>
A new version of VMMaker was added to project VM Maker Inbox:<br>
<a href="http://source.squeak.org/VMMakerInbox/VMMaker.oscog-KenD.2519.mcz" rel="noreferrer" target="_blank">http://source.squeak.org/VMMakerInbox/VMMaker.oscog-KenD.2519.mcz</a><br>
<br>
==================== Summary ====================<br>
<br>
Name: VMMaker.oscog-KenD.2519<br>
Author: KenD<br>
Time: 6 February 2019, 4:07:54.509792 pm<br>
UUID: cd615691-dbfb-4024-9b1b-4f05f0ae7d01<br>
Ancestors: VMMaker.oscog-KenD.2518<br>
<br>
Non-register struct returns now work.<br>
[Needs corresponding vm]<br>
<br>
=============== Diff against VMMaker.oscog-KenD.2518 ===============<br>
<br>
Item was changed:<br>
  SystemOrganization addCategory: #'VMMaker-Building'!<br>
  SystemOrganization addCategory: #'VMMaker-Interpreter'!<br>
  SystemOrganization addCategory: #'VMMaker-InterpreterSimulation'!<br>
  SystemOrganization addCategory: #'VMMaker-InterpreterSimulation-Morphic'!<br>
  SystemOrganization addCategory: #'VMMaker-JIT'!<br>
  SystemOrganization addCategory: #'VMMaker-JITSimulation'!<br>
  SystemOrganization addCategory: #'VMMaker-Multithreading'!<br>
  SystemOrganization addCategory: #'VMMaker-Plugins'!<br>
  SystemOrganization addCategory: #'VMMaker-Plugins-FFI'!<br>
  SystemOrganization addCategory: #'VMMaker-Plugins-IOS'!<br>
  SystemOrganization addCategory: #'VMMaker-PostProcessing'!<br>
  SystemOrganization addCategory: #'VMMaker-SmartSyntaxPlugins'!<br>
  SystemOrganization addCategory: #'VMMaker-SpurMemoryManager'!<br>
  SystemOrganization addCategory: #'VMMaker-SpurMemoryManagerSimulation'!<br>
  SystemOrganization addCategory: #'VMMaker-Support'!<br>
  SystemOrganization addCategory: #'VMMaker-Tests'!<br>
  SystemOrganization addCategory: #'VMMaker-Translation to C'!<br>
+ SystemOrganization addCategory: #VMMaker!<br>
<br>
Item was changed:<br>
  ----- Method: ThreadedARM64FFIPlugin>>ffiCalloutTo:SpecOnStack:in: (in category 'callout support') -----<br>
  ffiCalloutTo: procAddr SpecOnStack: specOnStack in: calloutState<br>
        <var: #procAddr type: #'void *'><br>
        <var: #calloutState type: #'CalloutState *'><br>
        <var: #loadFloatRegs declareC: 'extern void loadFloatRegs(double, double, double, double, double, double, double, double)'><br>
        "Go out, call this guy and create the return value.  This *must* be inlined because of<br>
         the alloca of the outgoing stack frame in ffiCall:WithFlags:NumArgs:Args:AndTypes:"<br>
        | myThreadIndex atomicType floatRet intRet x1Ret |<br>
        <var: #floatRet type: #double><br>
        <var: #intRet type: #usqLong><br>
        <var: #x1Ret type: #usqLong><br>
        <inline: true><br>
        myThreadIndex := interpreterProxy disownVM: (self disownFlagsFor: calloutState).<br>
<br>
-       "If struct address used, place it in x8"<br>
-       (calloutState structReturnSize > 0<br>
-        and: [(self returnStructInRegisters: calloutState structReturnSize) not]) ifTrue:<br>
-               [self setReturnRegister: calloutState limit]. "stack alloca'd struct"<br>
- <br>
        calloutState floatRegisterIndex > 0 ifTrue:<br>
                [self loadFloatRegs:<br>
                           (calloutState floatRegisters at: 0)<br>
                        _: (calloutState floatRegisters at: 1)<br>
                        _: (calloutState floatRegisters at: 2)<br>
                        _: (calloutState floatRegisters at: 3)<br>
                        _: (calloutState floatRegisters at: 4)<br>
                        _: (calloutState floatRegisters at: 5)<br>
                        _: (calloutState floatRegisters at: 6)<br>
                        _: (calloutState floatRegisters at: 7)].<br>
<br>
        (self allocaLiesSoSetSpBeforeCall or: [self mustAlignStack]) ifTrue:<br>
                [self setsp: calloutState argVector].<br>
<br>
        atomicType := self atomicTypeOf: calloutState ffiRetHeader.<br>
        (atomicType >> 1) = (FFITypeSingleFloat >> 1) ifTrue:<br>
                [atomicType = FFITypeSingleFloat<br>
                        ifTrue:<br>
                                [floatRet := self <br>
                                        dispatchFunctionPointer: (self cCoerceSimple: procAddr to: 'float (*)(sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t)')<br>
                                        with: (calloutState integerRegisters at: 0)<br>
                                        with: (calloutState integerRegisters at: 1)<br>
                                        with: (calloutState integerRegisters at: 2)<br>
                                        with: (calloutState integerRegisters at: 3)<br>
                                        with: (calloutState integerRegisters at: 4)<br>
                                        with: (calloutState integerRegisters at: 5)<br>
                                        with: (calloutState integerRegisters at: 6)<br>
                                        with: (calloutState integerRegisters at: 7)]<br>
                        ifFalse: "atomicType = FFITypeDoubleFloat"<br>
                                [floatRet := self <br>
                                        dispatchFunctionPointer: (self cCoerceSimple: procAddr to: 'double (*)(sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t)')<br>
                                        with: (calloutState integerRegisters at: 0)<br>
                                        with: (calloutState integerRegisters at: 1)<br>
                                        with: (calloutState integerRegisters at: 2)<br>
                                        with: (calloutState integerRegisters at: 3)<br>
                                        with: (calloutState integerRegisters at: 4)<br>
                                        with: (calloutState integerRegisters at: 5)<br>
                                        with: (calloutState integerRegisters at: 6)<br>
                                        with: (calloutState integerRegisters at: 7)].<br>
<br>
                 "undo any callee argument pops because it may confuse stack management with the alloca."<br>
                 (self isCalleePopsConvention: calloutState callFlags) ifTrue:<br>
                        [self setsp: calloutState argVector].<br>
                 interpreterProxy ownVM: myThreadIndex.<br>
<br>
                 ^interpreterProxy floatObjectOf: floatRet].<br>
<br>
+       "If struct address used for return value, call is special"<br>
+       (self mustReturnStructOnStack: calloutState structReturnSize) <br>
+       ifTrue: [<br>
+               intRet := 0.<br>
+               self setReturnRegister: (self cCoerceSimple: calloutState limit to: 'sqLong') "stack alloca'd struct"<br>
+                        andCall: (self cCoerceSimple: procAddr to: 'sqLong')<br>
+                        withArgsArray: (self cCoerceSimple: (self addressOf: calloutState integerRegisters) to: 'sqLong').<br>
+       ] ifFalse: [<br>
+               intRet := self <br>
-       intRet := self <br>
                                dispatchFunctionPointer: (self cCoerceSimple: procAddr to: 'usqIntptr_t (*)(sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t)')<br>
                                with: (calloutState integerRegisters at: 0)<br>
                                with: (calloutState integerRegisters at: 1)<br>
                                with: (calloutState integerRegisters at: 2)<br>
                                with: (calloutState integerRegisters at: 3)<br>
                                with: (calloutState integerRegisters at: 4)<br>
                                with: (calloutState integerRegisters at: 5)<br>
                                with: (calloutState integerRegisters at: 6)<br>
                                with: (calloutState integerRegisters at: 7).<br>
-       "Capture x1 immediately. No problem if unused"<br>
-        x1Ret := self getX1register.<br>
<br>
+        x1Ret := self getX1register. "Capture x1 immediately. No problem if unused"<br>
+       ].<br></blockquote><div><br></div><div>Have you considered the alternative, which would be to add an arm for structure return that would look something like</div><div><br></div><div>               structRet := self <br>                                dispatchFunctionPointer: (self cCoerceSimple: procAddr to: 'BigStruct (*)(sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t, sqIntptr_t)')<br>                                with: (calloutState integerRegisters at: 0)<br>                                with: (calloutState integerRegisters at: 1)<br>                                with: (calloutState integerRegisters at: 2)<br>                                with: (calloutState integerRegisters at: 3)<br>                                with: (calloutState integerRegisters at: 4)<br>                                with: (calloutState integerRegisters at: 5)<br>                                with: (calloutState integerRegisters at: 6)<br>                                with: (calloutState integerRegisters at: 7).<br></div><div>?</div><div><br></div><div>I guess the problem with this is that it only copies back as many bytes as we declare BigStruct has, and that can never be right.  If BigStruct is too small we don't copy back enough bytes and if BigStruct is very large we ri=sk a page fault when we copy too much.  And of course one can always declare a BigStruct bigger than the one we define.</div><div><br></div><div>So I agree with your approach.  I do want to see the issue adequately documented in the code though.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
        "If struct returned in registers, <br>
         place register values into calloutState integerRegisters"<br>
        (calloutState structReturnSize > 0<br>
         and: [self returnStructInRegisters: calloutState structReturnSize]) ifTrue: <br>
                ["Only 2 regs used in ARMv8/Aarch64 current"<br>
                 calloutState integerRegisters at: 0 put: intRet. "X0"<br>
                 calloutState integerRegisters at: 1 put: x1Ret]. "X1"<br>
<br>
        "undo any callee argument pops because it may confuse stack management with the alloca."<br>
        (self isCalleePopsConvention: calloutState callFlags) ifTrue:<br>
                [self setsp: calloutState argVector].<br>
        interpreterProxy ownVM: myThreadIndex.<br>
<br>
        (calloutState ffiRetHeader anyMask: FFIFlagPointer+FFIFlagStructure) ifTrue:<br>
                ["Note: Order is important here since FFIFlagPointer + FFIFlagStructure is used to represent<br>
                 'typedef void* VoidPointer' and VoidPointer must be returned as pointer *not* as struct."<br>
                 (calloutState ffiRetHeader anyMask: FFIFlagPointer) ifTrue:<br>
                        [^self ffiReturnPointer: intRet ofType: (self ffiReturnType: specOnStack) in: calloutState].<br>
                 ^self ffiReturnStruct: intRet ofType: (self ffiReturnType: specOnStack) in: calloutState].<br>
<br>
        ^self ffiCreateIntegralResultOop: intRet ofAtomicType: atomicType in: calloutState!<br>
<br>
Item was added:<br>
+ ----- Method: ThreadedARM64FFIPlugin>>mustReturnStructOnStack: (in category 'marshalling') -----<br>
+ mustReturnStructOnStack: returnStructSize<br>
+       "Answer if a struct result of a given size is unable to be returned in registers."<br>
+       ^returnStructSize > (2 * self wordSize)!<br>
<br>
Item was changed:<br>
  ----- Method: ThreadedARM64FFIPlugin>>returnStructInRegisters: (in category 'marshalling') -----<br>
  returnStructInRegisters: returnStructSize<br>
+       "Answer if a struct result of a given size is able to be returned in registers.<br>
+       NB: this is a predicate!! #returnStructInRegisters: does NOT return a struct in anything!!"<br>
-       "Answer if a struct result of a given size is able to be returned in registers."<br>
        ^returnStructSize <= (2 * self wordSize)!<br>
<br>
Item was removed:<br>
- ----- Method: ThreadedARM64FFIPlugin>>setReturnRegister: (in category 'callout support') -----<br>
- setReturnRegister: structAddr<br>
- <br>
-       <inline: true><br>
-       <var: #structAddr type: #'sqLong'><br>
-       <var: #setStructReturnAddressRegister declareC: 'extern void setStructReturnAddressRegister(sqLong structAddr)'><br>
- <br>
-       self setStructReturnAddressRegister: structAddr!<br>
<br>
Item was added:<br>
+ ----- Method: ThreadedARM64FFIPlugin>>setReturnRegister:andCall:withArgsArray: (in category 'callout support') -----<br>
+ setReturnRegister: structAddr andCall: procAddr withArgsArray: arrayAddr<br>
+ <br>
+       <inline: true><br>
+       <var: #structAddr type: #'sqLong'><br>
+       <var: #procAddr  type: #'sqLong'><br>
+       <var: #arrayAddr type: #'sqLong'><br>
+       <var: #callAndReturnWithStructAddr declareC: 'extern void callAndReturnWithStructAddr(sqLong structAddr,sqLong procAddr,sqLong arrayAddr)'><br>
+ <br>
+       self callAndReturnWithStructAddr: structAddr _: procAddr _: arrayAddr!<br>
<br>
Item was added:<br>
+ ServiceProvider subclass: #VMMakerServiceProvider<br>
+       instanceVariableNames: ''<br>
+       classVariableNames: ''<br>
+       poolDictionaries: ''<br>
+       category: 'VMMaker'!<br>
<br>
Item was added:<br>
+ ----- Method: VMMakerServiceProvider class>>initialize (in category 'initialization') -----<br>
+ initialize <br>
+       ServiceRegistry current buildProvider: self new!<br>
<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><span style="font-size:small;border-collapse:separate"><div>_,,,^..^,,,_<br></div><div>best, Eliot</div></span></div></div></div></div></div></div>