Fix RLSRA for intervals of length 1

src/Tinyrossa-Tests/TRCompilationTestCase.class.st

@@ -252,6 +252,78 @@ TRCompilationTestCase >> test03_lconst [
 	"
 ]
 
+{ #category : #tests }
+TRCompilationTestCase >> test06_iadd_discarding_value [
+	| x builder |
+
+	<parameter: #x values: #(0 10)>
+
+	x := testParameters at: #x.
+
+	builder := compilation builder.
+	builder defineName: testSelector asString type: Int32.
+	builder defineParameter: 'x' type: Int32.
+	builder treetop: {
+		builder iadd: {
+			builder iload: 'x'.
+			builder iconst: 1 } }.
+	builder ireturn:
+		{ builder iconst: -1 }.
+
+	compilation optimize.
+	compilation compile.
+
+	self assert: (shell call: x ) equals: -1.
+
+	"
+	TRRV64GCompilationTests debug: #test06_iadd_discarding_value
+	TRPPC64CompilationTests debug: #test06_iadd_discarding_value
+	"
+]
+
+{ #category : #tests }
+TRCompilationTestCase >> test07_call_discarding_value [
+	| x builder |
+
+	<parameter: #x values: #(0 10)>
+
+	self target name = 'powerpc64le-linux' ifTrue: [
+		self skip: 'Skipped since Xload/Xstore evaluator does not support statics for POWER (see issue #52)'.
+	].
+
+	x := testParameters at: #x.
+
+	builder := compilation builder.
+	builder defineName: testSelector asString type: Int32.
+	builder defineParameter: 'x' type: Int32.
+	builder
+		if: (builder icmpeq:
+			 {  builder iload: 'x' .
+				builder iconst: 0  })
+		then:[ :builder |
+				builder ireturn:
+					{ builder iconst: -1 } ]
+		else:[ :builder |
+				builder icall: {
+					builder isub:
+						{ builder iload: 'x' .
+						  builder iconst: 1 } .
+					testSelector }.
+				builder ireturn:
+						{ builder iload: 'x' } ].
+
+	compilation optimize.
+	compilation compile.
+
+
+	self assert: (shell call: x) equals: ((x == 0) ifTrue:[ -1 ] ifFalse:[ x ]).
+
+	"
+	TRRV64GCompilationTests debug: #test07_call_discarding_value
+	TRPPC64CompilationTests debug: #test07_call_discarding_value
+	"
+]
+
 { #category : #'tests - examples' }
 TRCompilationTestCase >> test_example01_meaningOfLife [
 	TRCompilationExamples new

src/Tinyrossa-Tests/TRILSimplifierTests.class.st

@@ -117,11 +117,11 @@ TRILSimplifierTests >> test_simplify_store_02 [
 	b ireturn: {
 		b iconst: 2 }.
 
-	self assert: compilation cfg treetops size == 4.
-	self assert: compilation cfg treetops second opcode = istore.
+	self assert: compilation cfg treetops size == 5.
+	self assert: compilation cfg treetops third opcode = istore.
 
 	compilation optimize.
 
-	self assert: compilation cfg treetops size == 4.
-	self assert: compilation cfg treetops second opcode = treetop.
+	self assert: compilation cfg treetops size == 5.
+	self assert: compilation cfg treetops third opcode = treetop.
 ]

src/Tinyrossa/TRCompilationExamples.class.st

@@ -687,11 +687,7 @@ TRCompilationExamples >> example17_call_external_function_in_aot_mode [
 
 	(builder defineFunction: 'exit' type: Void). 
 
-	"Note, what we need to anchor call in a treetop otherwise
-	 it would not be referenced from tree. Future versions of IL builder
-	 may do this automatically."
-
-	builder treetop: { builder call: { builder iload: 'status' . 'exit' } }.
+	builder call: { builder iload: 'status' . 'exit' }.
 	builder ireturn: { builder iconst: 0 }.
 
 	compilation config

src/Tinyrossa/TRILBuilder.class.st

@@ -43,6 +43,19 @@ TRILBuilder >> build: opcode arguments: arguments [
 
 	node := super build: opcode arguments: arguments.
 	node location: location.
+
+	"If the node is call, we anchor it here under a treetop.
+	 This is neccessary because otherwise, the node would be
+	 elided if nobody uses the return value and we cannot elide
+	 calls because they may have sideeffects.
+
+	 If we really want to elide calls to sideffect-free functions
+	 (whateber that means) whose value are not used, it should be
+	 done as an optimization pass."
+	opcode isCall ifTrue: [
+		current add: (TRILNode opcode: treetop children: { node })
+	].
+
 	^ node
 ]
 

src/Tinyrossa/TRRegisterLiveInterval.class.st

@@ -99,6 +99,18 @@ TRRegisterLiveInterval >> firstDef [
 	^ nil
 ]
 
+{ #category : #accessing }
+TRRegisterLiveInterval >> firstUse [
+	"Return the first use position for this interval."
+
+	uses do: [:encodedPosition |
+		(self encodesUsePosition: encodedPosition) ifTrue: [
+			^ self decodePosition: encodedPosition
+		].
+	].
+	^ nil
+]
+
 { #category : #initialization }
 TRRegisterLiveInterval >> initializeWithRegister: aTRVirtualRegister [
 	self assert: aTRVirtualRegister isTRVirtualRegister.
@@ -172,15 +184,36 @@ TRRegisterLiveInterval >> length [
 	^ self stop - self start + 1
 ]
 
+{ #category : #queries }
+TRRegisterLiveInterval >> needsToBeSpilled [
+	"Return true, if this interval (register) has to be spilled after its definition"
+	^ spillSlot notNil
+]
+
 { #category : #queries }
 TRRegisterLiveInterval >> needsToBeSpilledAt: position [
 	^ spillSlot notNil and: [ position = self lastDef ]
 ]
 
-{ #category : #queries }
-TRRegisterLiveInterval >> needsToBeSpilled [
-	"Return true, if this interval (register) has to be spilled after its definition"
-	^ spillSlot notNil
+{ #category : #accessing }
+TRRegisterLiveInterval >> nextUseAfter: position [
+	"Return the next (closest) use position for this interval greater than `position`.
+	 If there's no use after given position and before next (closest) def position
+	 return `nil`."
+
+	| encodedDefPosition |
+
+	encodedDefPosition := self encodeDefPosition: position.
+	uses do: [ :i |
+		encodedDefPosition < i ifTrue:[
+			(self encodesUsePosition:i) ifTrue: [
+				^ self decodePosition: i.
+			] ifFalse: [
+				^ nil
+			].
+		].
+	].
+	^ nil.
 ]
 
 { #category : #'printing & storing' }

src/Tinyrossa/TRReverseLinearScanRegisterAllocator.class.st

@@ -142,7 +142,9 @@ TRReverseLinearScanRegisterAllocator >> allocateRegisters [
 		].
 	].
 	intervals do: [:interval | 
-		self assert: interval start < interval stop.
+		self assert: interval start <= interval stop.
+		self assert: interval firstDef notNil description: 'virtual register not defined (assigned)'.
+		self assert:(interval firstUse isNil or:[interval firstDef < interval firstUse]) description: 'virtual register used before defined (assigned)'.
 	].
 
 	"Create todo (work) list. The list is sorted by interval's end position (#stop).
@@ -191,12 +193,19 @@ TRReverseLinearScanRegisterAllocator >> allocateRegistersAt: insnIndex [
 
 		deps post do: [:dep |
 			dep isUnsatisfiedDependency ifTrue:[
-				"Move value from real register to its virtual register."
-				self insertMoveFrom: dep rreg to: dep vreg.
+				"Move value from real register to its virtual register but only
+				 if the value is needed (this is usually the case, but not always!)"
+				| interval |
+
+				interval := live detect: [:each | each register == dep vreg ] ifNone: [ nil ].
+				(interval notNil and:[(interval nextUseAfter: insnIndex) notNil]) ifTrue: [
+					self insertMoveFrom: dep rreg to: dep vreg.
+				].
 
 				live copy do: [:i |
 					(i register allocation == dep rreg) ifTrue: [
-						"Live-across register is trashed, we have to spill and reload.
+						"If any live register is allocated to dependency's real register
+						 (which is trashed at this point) we have to spill and reload.
 						 We do it by forcefully splitting the interval."
 						self splitRegister: i at: insnIndex.
 					].
@@ -229,14 +238,17 @@ TRReverseLinearScanRegisterAllocator >> allocateRegistersAt: insnIndex [
 		self allocateRegister: interval.
 		interval length == 1 ifTrue: [ 
 			"We have just allocated an register interval of length 1 -
-			 such interval may be result of a split between its 
-			 definition and first use. In this case, this interval is 
-			 defined at this interval get immediatelly spilled so
-			 we can expire it right now to free the register for
-			 possibly other intervals that go live here."
-			self assert: (interval needsToBeSpilledAt: insnIndex).
-
-			self insertSpill: interval.
+			 such interval may be result certain TRIL (like a call to
+			 non-void function whose return value is not used) or it may
+			 be result of a split between its definition and first use.
+			 If the latter, this interval is defined at this interval
+			 must be immediatelly spilled.
+
+			 In both cases, so we can expire it right now to free the
+			 register for possibly other intervals that go live here."
+			(interval needsToBeSpilledAt: insnIndex) ifTrue: [
+				self insertSpill: interval.
+			].
 			self expireRegister: interval.
 		]
 	].