[Vm-dev] Re: [squeak-dev] VM 4.4 build nastinesses - part 1 of probably many

[tim Rowledge]

On 12-02-2013, at 12:30 PM, Eliot Miranda <eliot.miranda at gmail.com> wrote:
> > Can anyone explain why totally inappropriate and highly OS dependant code like push/popOutputFile() are in CROSS/sqVirtualMachine.c? The definition of stdin/out/err is not a safe one to assume everywhere. Not every OS trivially maps a FILE* to a number.
> 
> Well, this is really useful for debugging and I was under the impression that the stdio facilities used here-in are part of the standard C library.  Personally I'd much rather see the code guarded by an ifdef excluding RISCOS than I would see three identical copies of the code.  The platform code is a mess of duplications, and whereeever possible I've been putting common code in Cross.


OK, fair point mostly. fopen/fclose etc are stdio *but* fileno() is not. The assumption of stin/out/err being 0/1/2 etc is not stdio.
I'm pretty sure the same functionality could be done properly portably, or maybe better yet with some explanatory comments in the code it would be easy to understand the intent and hook up to either a portable implementation or a platform specific one as preferred. For example, I would probably benefit more on RISC OS by hooking to !SysLog which is part of the OS support than a poorly support unix idea of output streams.

Whether something is considered Cross or platform can be a tricky choice. If Fwiblle(thing (*burp)(long float far belch)) is useful on 3 out of 4 platforms, does that suit it to be considered Cross? Yeah, maybe. What if a couple of new platforms come along that do or don't like it? I guess we debate it and the most persistent one 'wins'.


> BTW, this is a pet peeve of mine.  Not only is there significant duplication of code, there is egregious difference.  e.g. we have platforms/Mac OS/vm/sqMacMain.c, platforms/unix/vm/sqUnixMain.c and platforms/win32/vm/sqWin32Intel.c, whereas platforms/Mac OS/vm/sqMain.c, platforms/unix/vm/sqMain.c and platforms/win32/vm/sqMain.c would be far more rational and easy to search.
> 
> So IMO that code is better placed in sqVirtualMachine.c than elsewhere, and the entire platform hierarchy could do with some serious renaming.

No arguments at all about the disparity in naming files being a pain BUT you need to remember or hear some ancient history to understand the why. Not to mention the why of many VMMaker strangenesses. I'll explain some of that in a separate email in a while.

> 
> >
> > It looks like the primitiveFunctionPointer global got changed fro mvoid* in 4.0 to void(*)() in 4.4. This is a good thing *if* the matching usages are correctly modified; which they aren't. For example in primitiveExecuteMethodArgsArray(). On my compiler that is a fatal error - as it should be. Is anyone currently fixing that or shall I?
> 
> Please do, or send me a pointer.

Well, I'm dinking with it in the plain interpreter, mostly as an exercise in self-abuse. I noticed in a couple of places in the stack vm code included within VMMaker-dtl.278 that you've already got some corrections in place.
>  
> Possibly as a side effect primitiveFunctionPointer is no longer in the global struct, which may well adversely impact performance.
> 
> Two issues here.  a) if it's excluded from the global struct as a a side-effect it's a bug. b) you need to run a performance comparison to verify whether the struct works well or not.  For x86 it does not.  Note that in the Cog branch I went to so,e effort to allow you to select which variant (struct or flat) at compile time instead of VM generation time.
> 

I suspect it might be excluded from the global struct simply because I recall it being simpler Back Then to leave out anything that was declared non-integer-ish; arrays etc. So changing it to sqInt (*)(void) would likely hit that unless someone has made stuff cleverer. It looks in interp.c that it's just non-foo right now.

As for being good for x86 or not I wouldn't have a clue. The global struct is completely optional and I only ever used it for ARM; it looks like IanP and John McI concluded it was of benefit at some point. If it's no longer true someone should change it. It's potentially irrelevant given the availability of cog for x86.



> 
> And WELCOME BACK!!!!

Ta.


tim
--
tim Rowledge; tim at rowledge.org; http://www.rowledge.org/tim
Strange OpCodes: FCE: Fill Core with Epoxy