Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mixed-arithmetic handling only works for float+int, not int+float #669

Open
marceltaeumel opened this issue Dec 7, 2023 · 3 comments
Open

Comments

@marceltaeumel
Copy link
Contributor

marceltaeumel commented Dec 7, 2023

I played around with VM parameter 75. It does indeed show a change for the float-based primitives (i.e., 541, 542, ...) but not the integer-based primitives (i.e., 1, 2, 3, ...).

Is this expected behavior? In the VMMaker sources, it looks like it should work at least for comparisons (i.e., 3 to 8):

genPrimitiveLessThan
	^coInterpreter primitiveDoMixedArithmetic
		ifTrue: [self
				genSmallIntegerComparison: JumpLess
				orDoubleComparison: #JumpFPGreater:
				invert: true]
		ifFalse: [self genSmallIntegerComparison: JumpLess]

Is genPrimitiveLessThan not the one in charge here? Hm....

So, debugging 3+4.0 shows that the primitive will fail, which it should not when mixed-arithmetic would work as expected.

@nicolas-cellier-aka-nice
Copy link
Contributor

nicolas-cellier-aka-nice commented Dec 7, 2023

I don't remember, there are many possible path, what if not Jitted ?
Certainly a case for exercising VM Simulator skills, or learn them...

@nicolas-cellier-aka-nice
Copy link
Contributor

nicolas-cellier-aka-nice commented Dec 7, 2023

Note that the primitiveLessThan should fail: it only handle SmallInteger argument (and receiver).

It's bytecodePrimLessThan that eventually handle the automatic int -> float conversion.

Remember that we (the stack interpreter) short-circuit some message sends for some specialSelectors when we can recognize known operand types.

Would it be possible that the debugger fails to emulate the special send machinery?

@nicolas-cellier-aka-nice
Copy link
Contributor

nicolas-cellier-aka-nice commented Dec 11, 2023

It appears that the JIT does thing differently than the (stack) interpreter.

The interpreter will try things in this order when interpreting the send of a special selector for arithmetic or comparison - for example in bytecodePrimAdd

  • if both receiver and argument are SmallInteger, then perform the operation immediately
  • else try to perform corresponding float primitive (primitiveFloatAdd:toArg:) that will eventually attempt a conversion sqInt -> double through loadFloatOrIntFrom:
  • if it fails, then fallback to a message send, which will normally call primitiveAdd if receiver is a SmallInteger (and fail for the same reason as first case)

In this scheme, if we add handling of conversions in SmallInteger primitives, we will just duplicate the work done in the 2nd stage of bytecodePrimAdd for no added value, but just a few more wasted cycles.

The JIT will only try to handle the case of both SmallIntegers

  • in genSpecialSelectorArithmetic which is invoked for some of the specialSelectors (+ - bitAnd: bitOr:)
  • and genSpecialSelectorComparison (< > <= >= = ~=)

Then it will fallback to a message send which goes through the primitives.

and the integer primitives effectively do not handle conversion as we saw above.

The other way around, Float op: Integer, the float primitives do handle the conversion, and the JIT appears to be faster than Integer op: Float. The irony is that Jitted Integer op: Float is even slower than interpreted code.

I let Eliot explain while the second stage was skipped, but it might be that it consumes two many instructions: remember that the purpose is to inline the specialSelectors send, so we might end up with lot of instructions... If it doesn't pay statistically (except in micro benchmarks), then it's not worth it, because it will exhaust the space devoted to jitted code more rapidly (and remember, specialSelectors are sent from many places !).

We saw that adding conversions in integer primitives is not ideal, at least for the interpreted code.

So welcome to brighter ideas for having your cake and eat it too...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants