RLSRA: fix edge case when (virtual) register is only defd but not used

Consider following - somewhat pathological - TRIL fragment:

    N...
    N004 treetop
    N003   iadd
    N001     iload 'x'
    N002     iload 'y'
    N...

such that the value of the `iadd` node (`N003`) is never used. The iadd
node is compiled to (say) something like:

    addi {vr003}, {vr001}, {vr002}

This instruction defines (assigns) a virtual register `{vr003}` but
since its value is never used, the interval for `{vr003}` of length 1
and such interval was *not* created by interval splitting.

Now consider even trickier example:

    N...
    N004 treetop
    N003   icall 'some_func'
    N...

such that value of the icall node (`N003`) is never used just like in
previous example (in other words: the code calls a function `some_func`
but does not use its return value - an entirely valid, if not common,
situation).
Here, the call instruction does define (say) virtual register `{vr003}`
by having a dependency of linkage-defined return register on `{vr003}`.
But since value of `{vr003}` is never used, there's no need to generate
register move from linkage-defined return register to `{vr003}`.

This commit fixes RLSRA for cases like these.

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/TRRegisterLiveInterval.class.st

@@ -184,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

@@ -193,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.
  ].
@@ -231,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.
  ]
  ].