Added test on extended store and pop

smalltalksrc/VMMaker/SimpleStackBasedCogit.class.st

@@ -295,6 +295,12 @@ SimpleStackBasedCogit >> ceShortCutTraceStore: aProcessorSimulationTrap [
 				into: (processor registerAt: ReceiverResultReg)]
 ]
 
+{ #category : 'accessing' }
+SimpleStackBasedCogit >> ceStoreContextInstVarTrampoline: anInteger [ 
+	<doNotGenerate>
+	ceStoreContextInstVarTrampoline := anInteger
+]
+
 { #category : 'compile abstract instructions' }
 SimpleStackBasedCogit >> compileFrameBuild [
 	"Build a frame for a CogMethod activation.  See CoInterpreter class>>initializeFrameIndices.
@@ -1036,6 +1042,12 @@ SimpleStackBasedCogit >> evaluateTrampolineCallBlock: block protectLinkRegIfNot:
 			[ block value ].
 ]
 
+{ #category : 'accessing' }
+SimpleStackBasedCogit >> extA: anInteger [ 
+	<doNotGenerate>
+	extA := anInteger
+]
+
 { #category : 'bytecode generators' }
 SimpleStackBasedCogit >> extendedPushBytecode [
 	| variableType variableIndex |

smalltalksrc/VMMaker/StackToRegisterMappingCogit.class.st

@@ -1680,11 +1680,13 @@ StackToRegisterMappingCogit >> genForwardersInlinedIdenticalOrNotIf: orNot [
 
 { #category : 'bytecode generator stores' }
 StackToRegisterMappingCogit >> genGenericStorePop: popBoolean MaybeContextSlotIndex: slotIndex needsStoreCheck: needsStoreCheck needsRestoreRcvr: needsRestoreReceiver needsImmutabilityCheck: needsImmCheck [
+
 	"Generates a store into an object that *may* be a context.
 	Multiple settings:
 	- needsStoreCheck (young into old object check)
 	- needRestoreRcvr (ensures the recevier is live across the store)
 	- needsImmCheck (do the call-back if the receiver is immutable)"
+
 	<inline: true>
 	<var: #mutableJump type: #'AbstractInstruction *'>
 	<var: #immutabilityFailure type: #'AbstractInstruction *'>
@@ -1693,32 +1695,36 @@ StackToRegisterMappingCogit >> genGenericStorePop: popBoolean MaybeContextSlotIn
 	 involve wholesale reorganization of stack pages, and the only way to preserve the
 	 execution state of an activation in that case is if it has a frame."
 	self assert: needsFrame.
-	self 
-		cppIf: IMMUTABILITY
-		ifTrue:
-			[needsImmCheck
-				ifTrue: 
-					[mutableJump := objectRepresentation genJumpMutable: ReceiverResultReg scratchReg: TempReg.
-					 objectRepresentation genStoreTrampolineCall: slotIndex.
-					 needsRestoreReceiver ifTrue: [ self putSelfInReceiverResultReg ].
-					 immutabilityFailure := self Jump: 0.
-					 mutableJump jmpTarget: self Label.]].
-	self ssPop: 1.
-	self ssAllocateCallReg: ClassReg and: SendNumArgsReg. "for ceStoreContextInstVarTrampoline"
-	self ssPush: 1.
+
+	self ssAllocateRequiredReg: ClassReg upThrough: simStackPtr - 1. "If already classReg don't spill it"
+	"we replace the top value for the flush"
+	self ssStoreAndReplacePop: popBoolean toReg: ClassReg.
+	self ssFlushTo: simStackPtr.
+
+	self cppIf: IMMUTABILITY ifTrue: [ 
+		needsImmCheck ifTrue: [ 
+			mutableJump := objectRepresentation
+				               genJumpMutable: ReceiverResultReg
+				               scratchReg: TempReg.				
+			objectRepresentation genStoreTrampolineCall: slotIndex.
+
+			needsRestoreReceiver ifTrue: [ self putSelfInReceiverResultReg ].
+			immutabilityFailure := self Jump: 0.
+			mutableJump jmpTarget: self Label ] ].
+
 	objectRepresentation
 		genLoadSlot: SenderIndex
 		sourceReg: ReceiverResultReg
 		destReg: TempReg.
-	self ssStoreAndReplacePop: popBoolean toReg: ClassReg.
-	self ssFlushTo: simStackPtr.
+
 	self MoveCq: slotIndex R: SendNumArgsReg.
 	self CallRT: ceStoreContextInstVarTrampoline.
-	self 
+
+	self
 		cppIf: IMMUTABILITY
-		ifTrue:
-			[needsImmCheck ifTrue:[immutabilityFailure jmpTarget: self Label]].
-	^0
+		ifTrue: [ 
+		needsImmCheck ifTrue: [ immutabilityFailure jmpTarget: self Label ] ].
+	^ 0
 ]
 
 { #category : 'bytecode generator stores' }
@@ -4582,15 +4588,22 @@ StackToRegisterMappingCogit >> ssSelfDescriptor [
 
 { #category : 'simulation stack' }
 StackToRegisterMappingCogit >> ssStoreAndReplacePop: popBoolean toReg: reg [
-	"In addition to ssStorePop:toReg:, if this is a store and not
-	a popInto I change the simulated stack to use the register 
-	for the top value"
+
+	"Move the top of the stack to a register.
+	If the top of the stack must be popped, pop it.
+	If the top of the stack must NOT be popped, and was in memory (spilled), make it into the register"
+
 	| topSpilled |
 	topSpilled := self ssTop spilled.
-	self ssStorePop: (popBoolean or: [topSpilled]) toReg: reg.
-	popBoolean ifFalse: 
-		[ topSpilled ifFalse: [self ssPop: 1].
-		self ssPushRegister: reg ].
+	self ssStorePop: (popBoolean or: [ topSpilled ]) toReg: reg.
+
+	"If popBoolean is true, we already popped, do not pop again"
+	popBoolean ifTrue: [ ^ self ].
+
+	"If the top was spilled, we popped it before and moved it to `reg`.
+	Now replace its entry by `reg`, making it *unspilled*"
+	topSpilled ifFalse: [ self ssPop: 1 ].
+	self ssPushRegister: reg
 ]
 
 { #category : 'simulation stack' }
@@ -4825,6 +4838,12 @@ StackToRegisterMappingCogit >> updateSimSpillBase [
 	self assert: (simSpillBase > simStackPtr or: [(self simStackAt: simSpillBase) spilled == false])
 ]
 
+{ #category :'accessing' }
+StackToRegisterMappingCogit >> useTwoPaths: aBoolean [ 
+	<doNotGenerate>
+	useTwoPaths := aBoolean
+]
+
 { #category : 'span functions' }
 StackToRegisterMappingCogit >> v4PushNilSize: aMethodObj numInitialNils: numInitialNils [
 	"77			01001101				Push false [* 1:true, 2:nil, 3:thisContext, ..., -N: pushExplicitOuter: N, N = Extend B]

smalltalksrc/VMMakerTests/VMStorePopTest.class.st

@@ -0,0 +1,93 @@
+Class {
+	#name : #VMStorePopTest,
+	#superclass : #VMStackToRegisterMappingCogitTest,
+	#category : #'VMMakerTests-JitTests'
+}
+
+{ #category : #running }
+VMStorePopTest >> jitOptions [
+
+	^ super jitOptions
+		  at: #IMMUTABILITY put: true;
+		  yourself
+]
+
+{ #category : #tests }
+VMStorePopTest >> testExtendedStoreAndPopIV1ImmutableObjectCallingConvention [
+
+	| instanceVariableToWrite stopAddress methodWithStoreCheck storeTrampoline |
+	instanceVariableToWrite := 1.
+
+	"Create an object with at least `instanceVariableToWrite` instance variables.
+	In memory, instance variables are 0-indexed so substract 1"
+	obj := self newObjectWithSlots: instanceVariableToWrite.
+	memory setIsImmutableOf: obj to: true.
+
+	"The receiver should be in a receiver register based on Cog's calling convention"
+	machineSimulator receiverRegisterValue: obj.
+
+	"Set an address as store check trampoline.
+	The bytecode below will jump to it if it is a old -> young store"
+	storeTrampoline := self compileTrampoline: [ 
+			stopAddress := cogit Stop ] named: #ceStoreTrampoline.
+	cogit objectRepresentation
+		setAllStoreTrampolinesWith: storeTrampoline.
+
+	cogit ceStoreContextInstVarTrampoline: storeTrampoline.
+
+	"When the store trampoline is called, the assigned value should be in the class register.
+	Let's put a marker value. If we find this value after calling the trampoline, then the call did not set it."
+	machineSimulator classRegisterValue: 16rBADF00D.
+
+	"The first byte of the push receiver instance variable bytecode family is used to identify which variable (0-based again)"
+	cogit byte0: instanceVariableToWrite - 1.		
+	methodWithStoreCheck := self compile: [
+		cogit useTwoPaths: false.
+		cogit methodOrBlockNumTemps: 0.
+		cogit extA: 0.
+		cogit initSimStackForFramelessMethod: 0.
+		cogit byte1: 0.
+		cogit needsFrame: true.
+
+		cogit genPushLiteral: memory falseObject.
+		cogit genExtStoreReceiverVariableBytecode ].
+
+	self runFrom: methodWithStoreCheck until: stopAddress address.
+
+	self assert: machineSimulator classRegisterValue equals: memory falseObject
+]
+
+{ #category : #tests }
+VMStorePopTest >> testStoreAndPopIV1ImmutableObjectCallingConvention [
+
+	| instanceVariableToWrite stopAddress methodWithStoreCheck |
+	instanceVariableToWrite := 1.
+
+	"Create an object with at least `instanceVariableToWrite` instance variables.
+	In memory, instance variables are 0-indexed so substract 1"
+	obj := self newObjectWithSlots: instanceVariableToWrite.
+	memory setIsImmutableOf: obj to: true.
+
+	"The receiver should be in a receiver register based on Cog's calling convention"
+	machineSimulator receiverRegisterValue: obj.
+
+	"Set an address as store check trampoline.
+	The bytecode below will jump to it if it is a old -> young store"
+	cogit objectRepresentation
+		setAllStoreTrampolinesWith: (self compileTrampoline: [ 
+			stopAddress := cogit Stop ] named: #ceStoreTrampoline).
+
+	"The first byte of the push receiver instance variable bytecode family is used to identify which variable (0-based again)"
+	cogit byte0: instanceVariableToWrite - 1.		
+	methodWithStoreCheck := self compile: [
+		cogit useTwoPaths: false.
+		cogit methodOrBlockNumTemps: 0.
+		cogit initSimStackForFramelessMethod: 0.
+
+		cogit genPushLiteral: memory falseObject.
+		cogit genStoreAndPopReceiverVariableBytecode ].
+
+	self runFrom: methodWithStoreCheck until: stopAddress address.
+
+	self assert: machineSimulator classRegisterValue equals: memory falseObject
+]