Merge pull request #425 from pharo-project/backport-367

The store error code bytecode is skipped in both interpreted and compiled execution
pharo-project · May 30, 2022 · faea2cb · faea2cb
2 parents eda1de4 + d626262
commit faea2cb
Show file tree

Hide file tree

Showing 6 changed files with 1,053 additions and 851 deletions.
diff --git a/smalltalksrc/VMMaker/CoInterpreter.class.st b/smalltalksrc/VMMaker/CoInterpreter.class.st
@@ -533,7 +533,7 @@ CoInterpreter >> activateCoggedNewMethod: inInterpreter [
 
 	((self methodHeaderHasPrimitive: methodHeader)
 	 and: [primFailCode ~= 0]) ifTrue:
-		[self reapAndResetErrorCodeTo: stackPointer header: methodHeader].
+		[self reapAndResetErrorCodeTo: framePointer header: methodHeader].
 
 	"Now check for stack overflow or an event (interrupt, must scavenge, etc)."
 	stackPointer >= stackLimit ifTrue:
@@ -769,8 +769,10 @@ CoInterpreter >> activateNewMethod [
 		["Skip the CallPrimitive bytecode, if it's there, and store the error code if the method starts
 		  with a long store temp.  Strictly no need to skip the store because it's effectively a noop."
 		 instructionPointer := instructionPointer + (self sizeOfCallPrimitiveBytecode: methodHeader).
-		 primFailCode ~= 0 ifTrue:
-			[self reapAndResetErrorCodeTo: stackPointer header: methodHeader]].
+		 primFailCode ~= 0 ifTrue: [ | shouldSkipStoreBytecode |
+			shouldSkipStoreBytecode := self reapAndResetErrorCodeTo: framePointer header: methodHeader.
+			shouldSkipStoreBytecode ifTrue: [ 
+				instructionPointer := instructionPointer + (self sizeOfLongStoreTempBytecode: methodHeader)  ] ] ].
 
 	"Now check for stack overflow or an event (interrupt, must scavenge, etc)."
 	switched := true.
@@ -1775,7 +1777,7 @@ CoInterpreter >> ceReapAndResetErrorCodeFor: cogMethod [
 	<var: #cogMethod type: #'CogMethod *'>
 	self assert: primFailCode ~= 0.
 	newMethod := cogMethod methodObject.
-	self reapAndResetErrorCodeTo: stackPointer + objectMemory wordSize
+	self reapAndResetErrorCodeTo: framePointer
 		header: cogMethod methodHeader
 ]
 
@@ -4115,8 +4117,10 @@ CoInterpreter >> internalActivateNewMethod [
 		["Skip the CallPrimitive bytecode, if it's there, and store the error code if the method starts
 		  with a long store temp.  Strictly no need to skip the store because it's effectively a noop."
 		 localIP := localIP + (self sizeOfCallPrimitiveBytecode: methodHeader).
-		 primFailCode ~= 0 ifTrue:
-			[self reapAndResetErrorCodeTo: localSP header: methodHeader]].
+		 primFailCode ~= 0 ifTrue: [ | shouldSkipStoreBytecode |
+			shouldSkipStoreBytecode := self reapAndResetErrorCodeTo: localFP header: methodHeader.
+			shouldSkipStoreBytecode ifTrue: [ 
+				localIP := localIP + (self sizeOfLongStoreTempBytecode: methodHeader)  ] ] ].
 
 	self assert: (self frameNumArgs: localFP) = argumentCount.
 	self assert: (self frameIsBlockActivation: localFP) not.
@@ -4405,8 +4409,10 @@ CoInterpreter >> justActivateNewMethod: mustBeInterpreterFrame [
 		  with a long store temp.  Strictly no need to skip the store because it's effectively a noop."
 		 cogMethod ifNil:
 			[instructionPointer := instructionPointer + (self sizeOfCallPrimitiveBytecode: methodHeader)].
-		 primFailCode ~= 0 ifTrue:
-			[self reapAndResetErrorCodeTo: stackPointer header: methodHeader]].
+		 primFailCode ~= 0 ifTrue: [ | shouldSkipStoreBytecode |
+			shouldSkipStoreBytecode := self reapAndResetErrorCodeTo: framePointer header: methodHeader.
+			(cogMethod isNil and: [shouldSkipStoreBytecode]) ifTrue: [ 
+				instructionPointer := instructionPointer + (self sizeOfLongStoreTempBytecode: methodHeader)  ] ] ]..
 
 	^methodHeader
 ]
@@ -5624,18 +5630,6 @@ CoInterpreter >> primErrTable [
 	^objectMemory splObj: PrimErrTableIndex
 ]
 
-{ #category : #'cog jit support' }
-CoInterpreter >> primFailCode [
-	<doNotGenerate>
-	^primFailCode
-]
-
-{ #category : #'cog jit support' }
-CoInterpreter >> primFailCode: anInteger [
-	<doNotGenerate>
-	primFailCode := anInteger
-]
-
 { #category : #'trampoline support' }
 CoInterpreter >> primFailCodeAddress [
 	<api>

diff --git a/smalltalksrc/VMMaker/StackInterpreter.class.st b/smalltalksrc/VMMaker/StackInterpreter.class.st
@@ -8513,8 +8513,10 @@ StackInterpreter >> internalActivateNewMethod [
 		["Skip the CallPrimitive bytecode, if it's there, and store the error code if the method starts
 		  with a long store temp.  Strictly no need to skip the store because it's effectively a noop."
 		 localIP := localIP + (self sizeOfCallPrimitiveBytecode: methodHeader).
-		 primFailCode ~= 0 ifTrue:
-			[self reapAndResetErrorCodeTo: localSP header: methodHeader]].
+		 primFailCode ~= 0 ifTrue: [ | shouldSkipStoreBytecode |
+			shouldSkipStoreBytecode := self reapAndResetErrorCodeTo: localFP header: methodHeader.
+			shouldSkipStoreBytecode ifTrue: [ 
+				localIP := localIP + (self sizeOfLongStoreTempBytecode: methodHeader)  ] ] ].
 
 	self assert: (self frameNumArgs: localFP) = argumentCount.
 	self assert: (self frameIsBlockActivation: localFP) not.
@@ -9356,8 +9358,10 @@ StackInterpreter >> justActivateNewMethod: mustBeInterpreterFrame [
 		["Skip the CallPrimitive bytecode, if it's there, and store the error code if the method starts
 		  with a long store temp.  Strictly no need to skip the store because it's effectively a noop."
 		 instructionPointer := instructionPointer + (self sizeOfCallPrimitiveBytecode: methodHeader).
-		 primFailCode ~= 0 ifTrue:
-			[self reapAndResetErrorCodeTo: stackPointer header: methodHeader]].
+		 primFailCode ~= 0 ifTrue: [ | shouldSkipStoreBytecode |
+			shouldSkipStoreBytecode := self reapAndResetErrorCodeTo: framePointer header: methodHeader.
+			shouldSkipStoreBytecode ifTrue: [ 
+				instructionPointer := instructionPointer + (self sizeOfLongStoreTempBytecode: methodHeader)  ] ] ].
 
 	^methodHeader
 ]
@@ -11574,6 +11578,12 @@ StackInterpreter >> primFailCode [
 	^ primFailCode 
 ]
 
+{ #category : #'cog jit support' }
+StackInterpreter >> primFailCode: anInteger [
+	<doNotGenerate>
+	primFailCode := anInteger
+]
+
 { #category : #'primitive support' }
 StackInterpreter >> primitiveAccessorDepthForExternalPrimitiveMethod: methodObj [
 	^objectMemory integerValueOf:
@@ -13669,18 +13679,29 @@ StackInterpreter >> readableFormat: imageVersion [
 ]
 
 { #category : #'primitive support' }
-StackInterpreter >> reapAndResetErrorCodeTo: theSP header: methodHeader [
+StackInterpreter >> reapAndResetErrorCodeTo: theFP header: methodHeader [
+
 	"Assuming the primFailCode is non-zero, check if the method consumes the error code
-	 and if so, assign it through theSP.  Then zero the primFailCode.  This is infrequent code,
-	 so keep it out of the common path."
+	 and if so, assign it through theFP into the correct temporary variable.
+	Then zero the primFailCode.  This is infrequent code, so keep it out of the common path."
+
 	<var: 'theSP' type: #'char *'>
 	<inline: #never>
 	| initialPC |
 	self assert: primFailCode ~= 0.
-	initialPC := (self initialIPForHeader: methodHeader method: newMethod) + (self sizeOfCallPrimitiveBytecode: methodHeader).
-	(objectMemory byteAt: initialPC) = (self longStoreBytecodeForHeader: methodHeader) ifTrue:
-		[stackPages longAtPointer: theSP put: self getErrorObjectFromPrimFailCode].
-	 primFailCode := 0
+	initialPC := (self initialIPForHeader: methodHeader method: newMethod)
+	             + (self sizeOfCallPrimitiveBytecode: methodHeader).
+	(objectMemory byteAt: initialPC)
+	= (self longStoreBytecodeForHeader: methodHeader) ifTrue: [ 
+		| temporaryIndex |
+		temporaryIndex := objectMemory byteAt: initialPC + 1.
+		self
+			temporary: temporaryIndex
+			in: theFP
+			put: self getErrorObjectFromPrimFailCode.
+		^ true].
+	primFailCode := 0.
+	^ false
 ]
 
 { #category : #'internal interpreter access' }

diff --git a/smalltalksrc/VMMaker/StackInterpreterSimulator.class.st b/smalltalksrc/VMMaker/StackInterpreterSimulator.class.st
@@ -1284,6 +1284,12 @@ StackInterpreterSimulator >> localFP [
 	^localFP
 ]
 
+{ #category : #'spur bootstrap' }
+StackInterpreterSimulator >> localFP: anFP [
+
+	^ localFP := anFP
+]
+
 { #category : #'spur bootstrap' }
 StackInterpreterSimulator >> localIP [
 	^localIP