Potential Issue in BytesToWordsInPlace() Win64 asm for small strings

[Arnaud Bouchez]

In https://bitbucket.org/egrange/dwscript/commits/4009a9cf5e22c3e2144ac363bb1b205665d55095#LSource/dwsXPlatform.pasT1953

If lessthan16 branch is taken, then ah register is never set so you may just write garbage in the output buffer I am afraid.

Some notes about writing such asm:

• For such small functions, .noframe or nostackframe; on FPC reduces a bit the code;
• You can use parameter names in the asm, i.e. directly p and n directly within the code, so that it would also work on POSIX ABI - but take care that you should take care by hand that the other registers used within the function don’t conflict with the parameters on all ABIs;
• IIRC dec has a performance penalty on x86_64 so sub xxx, 1 may be preferred - from Agner Fog optimization guide.

‌

[Eric Grange]

Fixed Issue #220 in BytesToWordsInPlace + test

→ <<cset 9a99adf414b5>>

[Eric Grange]

Thanks, for the report!

DEC vs SUB had no measurable effect AFAICT, but there was an issue with buffers larger than 4GB which it brought to my attention. I have been shying away from using parameter names in asm, as outside very short routines, it indeed tends to obfuscate register conflicts.

FreePascal support is currently in stasis, because of issues in the FPC RTL around utf-16 support and variants, as FPC RTL is primarily an utf-8 world. It will be resumed when I can fully eliminate Variants in the engine, or replace them with a cross platform alternative.
Delphi Linux support is also non-existant yet, as I do not have access to that compiler.

[Arnaud Bouchez]

Thanks for the news.

Sad to hear about FPC support staling - what is the UTF-16 issue you are facing?
Of course, I don’t have such issue with mORMot, which is also in this UTF-8 dreamworld.

I am not surprised about Delphi Linux. I don’t have access to it either. And I was told that the generated code is not good.

Getting rid of Variants could be good for sure. Do you plan to cast values into 64-bit double, as most JS engines do?

[Eric Grange]

Utf-16 issues are that while FPC variants can store utf-16, but most of the RTL doesn’t support those, which leads to crashes and memory corruption. There is also the side issue that all the Delphi-like RTL classes are utf-8 rather than utf-16, and the compiler switch to use utf-16 by default has not been used/debugged much yet AFAICT.

Switching to utf-8 would be simpler, but it wouldn’t be break compatibility with existing scripts…

To get rid of variants, I’m replacing variants with strongly typed storage (f.i. the refactored dynamic arrays). And when impractical, 64bit storage yes, with the type stored separately (rather than type+data together like a variant).

It will come alongside redesigning the engine to be closure-oriented rather than stack-oriented.

Because of time constraints, all of this has to happen incrementally without breaking scripts… so I’m primarily stockpiling unit tests

[Arnaud Bouchez]

About values, I was more thinking about NAN boxing as in https://github.com/c-smile/quickjspp/blob/master/quickjs.h#L85
(which seems to me as a better alternative to the Int64 boxing used in original quickjs - IIRC this NAN boxing is what SpiderMonkey or V8 do)
Benefit is that it is still a 53-bit integer-like value, with easy access as double and that it has both value and type stored in 64-bit aligned data on all platforms.
With proper inlining of the access methods, it could be both efficient and memory saving.
Of course, it breaks the current expectations of DWS which is that integers have 64-bit resolution…

[Eric Grange]

The downside of that approach is that you can’t support 64bit integers… it is somewhat ok in JS, as JS never really supported integers.

However I think that’s an optimization gone too far, as it leads to weird “rounding” errors around 54bits when manipulating “integers” in JS.