[Vm-dev] [commit][3673] Provide VM hooks for testing Integer
class in plugins
Nicolas Cellier
nicolas.cellier.aka.nice at gmail.com
Wed Apr 20 23:19:18 UTC 2016
2016-04-20 23:50 GMT+02:00 Eliot Miranda <eliot.miranda at gmail.com>:
>
> Hi Nicolas, Hi All,
>
>
> Nicolas, I'm going to have to roll this form of the change back. The
> change ignores the versioning convention for the VirtualMachine struct in
> sqVirtualMachine.[ch]. If you look at the code you see things like:
>
> typedef struct VirtualMachine {
> sqInt (*minorVersion)(void);
> sqInt (*majorVersion)(void);
>
> /* InterpreterProxy methodsFor: 'stack access' */
>
> sqInt (*pop)(sqInt nItems);
> sqInt (*popthenPush)(sqInt nItems, sqInt oop);
> sqInt (*push)(sqInt object);
> sqInt (*pushBool)(sqInt trueOrFalse);
> sqInt (*pushFloat)(double f);
> sqInt (*pushInteger)(sqInt integerValue);
> double (*stackFloatValue)(sqInt offset);
> sqInt (*stackIntegerValue)(sqInt offset);
> sqInt (*stackObjectValue)(sqInt offset);
> sqInt (*stackValue)(sqInt offset);
> ...
> #if VM_PROXY_MINOR > 1
>
> /* InterpreterProxy methodsFor: 'BitBlt support' */
>
> sqInt (*loadBitBltFrom)(sqInt bbOop);
> sqInt (*copyBits)(void);
> sqInt (*copyBitsFromtoat)(sqInt leftX, sqInt rightX, sqInt yValue);
>
> #endif
> ...
> #if VM_PROXY_MINOR > 12 /* Spur */
> sqInt (*isImmediate)(sqInt objOop);
> sqInt (*characterObjectOf)(int charCode);
> sqInt (*characterValueOf)(sqInt objOop);
> sqInt (*isCharacterObject)(sqInt objOop);
> sqInt (*isCharacterValue)(int charCode);
> sqInt (*isPinned)(sqInt objOop);
> sqInt (*pinObject)(sqInt objOop);
> sqInt (*unpinObject)(sqInt objOop);
> #endif
> } VirtualMachine;
>
> So the idea is that the struct is changed only by changing the version
> number for the interface, and only adds new fields, and doesn't redefine
> existing ones. The idea is that existing external plugins that use older
> versions of the API all continue to work when loaded by VMs that support
> newer versions, but that (of course) newer versions of plugins won;t work
> with older VMs. The change above introduced new fields in the middle of
> the struct which breaks existing plugins.
>
> So in adding the new functions we want for the LargeIntegers plugin (and
> potentially others), we have to tread carefully and we have at least two
> choices.
>
> The functions we want to add are
>
> sqInt (*isKindOfInteger)(sqInt oop);
> sqInt (*isLargeIntegerObject)(sqInt oop);
> sqInt (*isLargePositiveIntegerObject)(sqInt oop);
> sqInt (*isLargeNegativeIntegerObject)(sqInt oop);
>
> or rather, in plugin code we want to be able to write
>
> isKindOfInteger(foo)
>
> etc.
>
> So we can either create a new version of the VirtualMachine API and add
> these functions at the end of the struct or we can try and implement them
> in terms of the existing API. If we add them at the end of the struct it
> looks like
>
> #if VM_PROXY_MINOR > 13
> sqInt (*isKindOfInteger)(sqInt oop);
> sqInt (*isLargeIntegerObject)(sqInt oop);
> sqInt (*isLargePositiveIntegerObject)(sqInt oop);
> sqInt (*isLargeNegativeIntegerObject)(sqInt oop);
> #endif
>
>
Hi Eliot,
OK, I see it now, obviously once compiled the names are gone, only the
position counts...
No use to break the old feature of providing binary compatibility indeed.
> But since we have all the functions we need to implement them already we
> don't need to dip this. my idea was to modify the plugin code generator to
> use the internal VM API when compiling a plugin as internal against Spur,
> but use the existing API for non-Spur or external plugins. The reason to
> use the special version for Spur is that in Spur the class indices of
> LargePositiveInteger and LargeNegativeInteger are known constants so we can
> generate simple and efficient code. I thought we could get it to output
> something like
>
> #if SPURVM && defined(SQUEAK_BUILTIN_PLUGIN)
> # define LargeNegativeIntegerClassIndex 32
> # define LargePositiveIntegerClassIndex 33
> extern sqInt classIndexOf(sqInt);
> # define isKindOfInteger(oop) (isImmediate(oop) ? isIntegerObject(oop) :
> (unsigned)(classIndexOf(oop) - LargeNegativeIntegerClassIndex) <= 1)
> # define isLargeIntegerObject(oop) (!isImmediate(oop)
> && (unsigned)(classIndexOf(oop) - LargeNegativeIntegerClassIndex) <= 1)
> # define isLargeNegativeIntegerObject(oop) (!isImmediate(oop) &&
> classIndexOf(oop) == LargeNegativeIntegerClassIndex)
> # define isLargePositiveIntegerObject(oop) (!isImmediate(oop) &&
> classIndexOf(oop) == LargePositiveIntegerClassIndex)
> #else
> # define isLargeNegativeIntegerObject(oop) (fetchClassOf(oop) ==
> LargeNegativeIntegerClassIndex)
> # define isLargePositiveIntegerObject(oop) (fetchClassOf(oop) ==
> LargePositiveIntegerClassIndex)
> # define isLargeIntegerObject(oop) (isLargeNegativeIntegerObject(oop)
> || isLargePositiveIntegerObject(oop))
> # define isKindOfInteger(oop) (isIntegerObject(oop)
> || isLargeNegativeIntegerObject(oop) || isLargePositiveIntegerObject(oop))
> #endif
>
> Does that make sense? Perhaps it's too tricky, but it's really nice /not/
> to have to change the VirtualMachine API often.
>
>
What wasn't clear in my mind was the else branch.
But yes, I see it better now, there's enough in the existing API to cover
these extra functions (maybe it shall be classLargePositiveInteger()
instead of LargePositiveIntegerClassIndex but that's details)
We can introduce such kind of "pseudo-API".
Another obstacle in my mind was the fact that the API is cloned only if
required (pluginFunctionsToClone).
We can make an exception for these "pseudo-API" and always generate them,
but that sounded quite arbitrary to me (not uniform).
Next thing that troubles me is why using macros for these selectors and not
for others.
For example, this one would be quite universal so far (even if not builtin):
#define isIntegerObject(oop) (oop & 1)
I tend to perceive non uniformity as extra complexity (using different
tricks for similar patterns).
My feeling is that we are trading extra-complexity for API stability.
On the other hand, fat API are not satisfying either.
It's a bit late here for me to continue anything tonight.
Revert what you want and tell me how I should proceed :)
cheers
> On Tue, Apr 19, 2016 at 5:06 PM, <commits at squeakvm.org> wrote:
>
>>
>> Revision: 3673
>> Author: nice
>> Date: 2016-04-19 17:06:30 -0700 (Tue, 19 Apr 2016)
>> Log Message:
>> -----------
>> Provide VM hooks for testing Integer class in plugins
>>
>> Modified Paths:
>> --------------
>> branches/Cog/platforms/Cross/vm/sqVirtualMachine.c
>> branches/Cog/platforms/Cross/vm/sqVirtualMachine.h
>>
>> Modified: branches/Cog/platforms/Cross/vm/sqVirtualMachine.c
>> ===================================================================
>> --- branches/Cog/platforms/Cross/vm/sqVirtualMachine.c 2016-04-19
>> 18:21:14 UTC (rev 3672)
>> +++ branches/Cog/platforms/Cross/vm/sqVirtualMachine.c 2016-04-20
>> 00:06:30 UTC (rev 3673)
>> @@ -65,7 +65,11 @@
>> sqInt isBytes(sqInt oop);
>> sqInt isFloatObject(sqInt oop);
>> sqInt isIndexable(sqInt oop);
>> -sqInt isIntegerObject(sqInt objectPointer);
>> +sqInt isKindOfInteger(sqInt oop);
>> +sqInt isLargeIntegerObject(sqInt oop);
>> +sqInt isLargePositiveIntegerObject(sqInt oop);
>> +sqInt isLargeNegativeIntegerObject(sqInt oop);
>> +sqInt isIntegerObject(sqInt oop);
>> sqInt isIntegerValue(sqInt intValue);
>> sqInt isPointers(sqInt oop);
>> sqInt isWeak(sqInt oop);
>> @@ -329,6 +333,10 @@
>> VM->isBytes = isBytes;
>> VM->isFloatObject = isFloatObject;
>> VM->isIndexable = isIndexable;
>> + VM->isKindOfInteger = isKindOfInteger;
>> + VM->isLargeIntegerObject = isLargeIntegerObject;
>> + VM->isLargePositiveIntegerObject = isLargePositiveIntegerObject;
>> + VM->isLargeNegativeIntegerObject = isLargeNegativeIntegerObject;
>> VM->isIntegerObject = isIntegerObject;
>> VM->isIntegerValue = isIntegerValue;
>> VM->isPointers = isPointers;
>>
>> Modified: branches/Cog/platforms/Cross/vm/sqVirtualMachine.h
>> ===================================================================
>> --- branches/Cog/platforms/Cross/vm/sqVirtualMachine.h 2016-04-19
>> 18:21:14 UTC (rev 3672)
>> +++ branches/Cog/platforms/Cross/vm/sqVirtualMachine.h 2016-04-20
>> 00:06:30 UTC (rev 3673)
>> @@ -111,7 +111,11 @@
>> sqInt (*isBytes)(sqInt oop);
>> sqInt (*isFloatObject)(sqInt oop);
>> sqInt (*isIndexable)(sqInt oop);
>> - sqInt (*isIntegerObject)(sqInt objectPointer);
>> + sqInt (*isKindOfInteger)(sqInt oop);
>> + sqInt (*isLargeIntegerObject)(sqInt oop);
>> + sqInt (*isLargePositiveIntegerObject)(sqInt oop);
>> + sqInt (*isLargeNegativeIntegerObject)(sqInt oop);
>> + sqInt (*isIntegerObject)(sqInt oop);
>> sqInt (*isIntegerValue)(sqInt intValue);
>> sqInt (*isPointers)(sqInt oop);
>> sqInt (*isWeak)(sqInt oop);
>>
>>
>
>
> --
> _,,,^..^,,,_
> best, Eliot
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20160421/edc83e19/attachment.htm
More information about the Vm-dev
mailing list