On Mon, May 13, 2024 at 8:54 AM Ignacio Truffat imtruffat@gmail.com wrote:
HI everybody,
@ElliotMiranda No need to apologize, just by looking at the github-repo I can see the colossal amount of work that went into that. On the contrary, I am trying to understand the building process as much as I can, so as to reduce the risk of finding a building error later on. (when I have made changes to the codebase) That's probably going to make debugging a lot easier, since I will be 99.99% sure the new bugs are something I introduced myself.
The good news is that I just tried to build an imagine with your recent change on my LinuxMint machine, and it seems to have worked! Don't want to jinx it, so I'll test this further before saying anything conclusive, but things seem to be going great for now! Finally I have a working sqcogspur64linuxht built from scratch after using ImageMaker to build the sources.
All and all, thank you very much for your swift response and fixes!
@TimeRowledge. You were correct, it seems that my machine was creating flaky code.
I was able to fix the clock issue (even before @ElliotMiranda's fix) by giving Squeak permission to run multi-threaded (explained here https://github.com/OpenSmalltalk/opensmalltalk-vm/releases/tag/r3732#linux), but then I kept making bad C code. In particular, The file cogitX64SysV.c was generated with invalid syntax, making labels appear within Ternary Conditional Operators.
for (numArgs = 0; numArgs < NumSendTrampolines; numArgs += 1) {
directedSuperSendTrampolines[numArgs] = (genSendTrampolineFornumArgscalledargargargarg(ceSendabovetonumArgs, numArgs, trampolineNamenumArgs("ceDirectedSuperSend", numArgs), ClassReg, TempReg, ReceiverResultReg, (numArgs <= (NumSendTrampolines - 2)? ( /* begin trampolineArgConstant: */ null, assert(numArgs >= 0), -2 - numArgs, l2: /* end trampolineArgConstant: */ ) : SendNumArgsReg)));
Found those bugs really interesting. I had never though much about the syntax of C itself (and how it was parsed), but it's true that labels are probably a very special case. I can upload those C files in case anybody finds this interesting to look into. However, looking further into those is not necessary as @ElliotMirandas's recent change has been able to skip the issue all together.
This is all to do with a rather flakey inliner. If you're interested look at TMethod>>#inlineSend:directReturn:exitVar:in: and other methods in the same protocol.
To investigate inlining in slang one can use the CCodeGenerator's inlining breakpoints. Seeimage/Slang Test Workspace.text
For example, to test that method you'd build a VMMaker for the Cogit and set the source breakpoint the the function being inlined (trampolineArgConstant:) and the destination to the method in which it's being inlined, generateSendTrampolines, so...
Transcript show: [| sel s vmm cg | sel *:=* #generateSendTrampolines. vmm *:=* VMMaker forPlatform: 'Cross'. cg *:=* [vmm interpreterClass: CoInterpreter; options: {#Cogit. Cogit chooseCogitClass name}; buildCodeGeneratorForCogit] on: Notification do: [:ex| ex tag == #getVMMaker ifTrue: [ex resume: vmm] ifFalse: [ex pass]].
*"to break at inlining decisions or type inference uncomment the following. If src & dest are different selectors, breaks on inlining. If src & dest are the same selector, breaks on type inference in sel."* cg breakSrcInlineSelector: #trampolineArgConstant:; breakDestInlineSelector: sel; breakOnInline: true. cg vmClass preGenerationHook: cg. cg inferTypesForImplicitlyTypedVariablesAndMethods. cg retainMethods: { sel }. cg prepareMethods. cg doInlining: cg vmClass doInlining. s *:=* ReadWriteStream on: String new. (cg methodNamed: sel) halt; emitCCodeOn: s generator: cg. s contents] value
This will break in the debugger when the code generator is deciding whether to inline trampolineArgConstant: into generateSendTrampolines:.
There are similar templates for interpreter methods, plugins, etc. Replacing hacky code with better, more comprehensible, more predictable, more stable code is of course a good thing, and I'm happy to work directly with people who want to work on Slang.
Kind regards, Ignacio Truffat
On Mon, May 13, 2024 at 4:42 AM Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Tim, Hi Ignacio,
Re initializeSimulationIOHighResClockForProfiling, it’s simulation time only and shouldn’t be generated. This is fixed in my latest commits.
Ignacio, I’m curious why you want your generate your own sources (and support you in doing so). Realise that Slang is a hack that has grown (metastasized??) over time. My attitude towards it is that it is better to suffer it and fix it while getting proper work done rather than rewriting it from scratch (in moving from 32 to 64 bits I had to do a lot of type inference work). So I apologise for its quality; at the moment it is part of the cost of doing business.
_,,,^..^,,,_ (phone)
On May 10, 2024, at 3:55 PM, Tim Rowledge tim@rowledge.org wrote:
On 2024-05-10, at 3:00 PM, Ignacio Truffat imtruffat@gmail.com
wrote:
This time, the main issue seems to that ioHighResClock variable, which
is probably defined in SOME C library. (so I am probably missing some dependency)
Not (normally) a variable - a function defined in the various
platforms/{platform name}/vm/sq{platform name}Heartbeat.c
for example - in platforms/unix/vm/sqUnixITimerHeartbeat.c ==================================== sqLong ioHighResClock(void) { /* return the value of the high performance counter */ sqLong value = 0;
#if (defined(__GNUC__) || defined(__SUNPRO_C)) && (defined(i386) ||
defined(__i386) || defined(__i386__))
__asm__ __volatile__ ("rdtsc" : "=A"(value)); #elif (defined(__GNUC__) || defined(__SUNPRO_C)) && (defined(x86_64) ||
defined(__x86_64) || defined (__x86_64__))
__asm__ __volatile__ ("rdtsc\n\t" // Returns the time in
EDX:EAX.
"shl $32, %%rdx\n\t" // Shift the upper bitsleft.
"or %%rdx, %0" // 'Or' in the lowerbits.
: "=a" (value) : : "rdx");#elif defined(__arm64__) || defined(__aarch64__) || defined(ARM64) __asm__ __volatile__ ("MRS X0, CNTVCT_EL0"); #elif defined(__arm__) && (defined(__ARM_ARCH_6__) ||
defined(__ARM_ARCH_7A__))
/* tpr - do nothing for now; needs input from eliot to decide
further */
/* Tim, not sure I have input beyond: Is there a 64-bit clock on ARM? If so, access it here :-) */ #elif defined(__riscv64__) __asm__ __volatile__ ("rdcycle a0" : "=r"(value)); #else # error "no high res clock defined" #endif return value; }
================================
Hmm, that's interesting, apparently I still owe Eliot info on a
high-res clock for ARM64. :-)
The function can just return 0 if you can't work out anything better.
Now it looks like your specific copy of cogitX64SysV.c is very
different to the standard one, which makes sense if I correctly remember anything at all about what you are trying to do.
Somewhere you have to declare that variable in a suitable for to assign
a... what? Is 'initializeSimulationIOHighResClockForProfiling()' returning a function pointer?
More generally it looks like the C code being generated is not quite
correct syntactically either.
=======
/home/mint2/Desktop/tesis/repo/opensmalltalk-vm/src/spur64.cog/cogitX64SysV.c:19662:11: error: expected ‘)’ before ‘:’ token
19662 | l6: /* end trampolineArgConstant: */) | ) ===========
tim
tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Strange OpCodes: BH: Branch and Hang