On 12 Oct 2015, at 3:43 , Henrik Johansen <henrik.s.johansen@veloxit.no> wrote:

On 10 Oct 2015, at 10:05 , Nicolai Hess <nicolaihess@web.de> wrote:

2015-10-08 23:45 GMT+02:00 Eliot Miranda <eliot.miranda@gmail.com>:

Hi Nicolai,

On Thu, Oct 8, 2015 at 2:27 PM, Nicolai Hess <nicolaihess@web.de> wrote:

Hello Eliot, and thanks for your time.

In our (pharo) implementation for sourceNodeForPC, we start at the AST-Node of

the compiled method,�� get the IR (IntermediateRepresentation) and scan this sequence of IRNodes

until we finally find that one, that maps to the bytecode of the closure creation code (this should be a IRPushClosureCopy).

The problem is now, we don't find (always) the right IRNode, and it depens on the number of local temps, if this is > 3 we don't hit the right one.

That's very strange.�� Can you construct a synthetic example of two simple blocks, one with three temps and and one with four temps and show me the difference in the byte code?

[ :x | | a b c | a:=b:=c:=x ]���������������� "calling #sourceNode gives a RBBlockNode - ok"

Bytecode
13 <8F 01 00 0B> closureNumCopied: 0 numArgs: 1 bytes 17 to 27
17 <73> pushConstant: nil
18 <73> pushConstant: nil
19 <73> pushConstant: nil
20 <10> pushTemp: 0
21 <81 43> storeIntoTemp: 3
23 <81 42> storeIntoTemp: 2
25 <81 41> storeIntoTemp: 1
27 <7D> blockReturn
28 <7C> returnTop

[ :x | | a b c d | a:=b:=c:=d:=x ]������ "calling #sourceNode gives a RBMethodNode - not ok"

Bytecode
13 <8F 01 00 0E> closureNumCopied: 0 numArgs: 1 bytes 17 to 30
17 <73> pushConstant: nil
18 <73> pushConstant: nil
19 <73> pushConstant: nil
20 <73> pushConstant: nil
21 <10> pushTemp: 0
22 <81 44> storeIntoTemp: 4
24 <81 43> storeIntoTemp: 3
26 <81 42> storeIntoTemp: 2
28 <81 41> storeIntoTemp: 1
30 <7D> blockReturn
31 <7C> returnTop

As I wrote above, in Pharo, to get the sourceNode we try to find the push closure bytecode from "startpc - 1" by traversing

the IR and try to get the IRNode with matching "bytecodeOffset". The bytecodeOffsets for the above blocks:

[ :x | | a b c | a:=b:=c:=x ]

"bytecodeOffset -> IR"

19->pushClosureCopyCopiedValues: #() args: #(#x)
28->returnTop
20->pushTemp: #x
22->storeTemp: #c
24->storeTemp: #b
26->storeTemp: #a
27->blockReturnTop

[ :x | | a b c d | a:=b:=c:=d:=x ]
"bytecodeOffset -> IR"
20->pushClosureCopyCopiedValues: #() args: #(#x)
31->returnTop
21->pushTemp: #x
23->storeTemp: #d
25->storeTemp: #c
27->storeTemp: #b
29->storeTemp: #a
30->blockReturnTop

Both blocks don't have a bytecodeOffset matching the bytecode of the pushclosure (13), but we start the search from "startpc - 1" (= 16)

and do a loop from 0 to -3 (because bytecodes can have a length of 1,2 or 4)

For the first block, this works

bytecodeOffset 19 = 16 - (-3)

For the second block this does not work, somehow the bytecodeoffset value depends on the number of local temps. And for the

second block this is one greater.

��

And my interpretation was, the IRPushClosureCopy node has the wrong "bytecode offset" (whatever this is). The bytecodeOffset value (somehow)

depends on the number of local temps, but maybe this is right (for whatever this is used) and we are just using it wrong for in this situation.

So you need to ask Marcus how to interpret bytecodeOffset.�� The thing is, it should be simple.�� One has:

�� �� BlockCreationBytecode

�� �� any number of pushConstant: nil's (0 to N)

�� �� first byte code in block

�� �� last bytecode in block (always a blockReturnTop)

and a block's startup is always that of the byte immediately following the BlockCreationBytecode.�� So if the byte code offset is somehow the span of the "any number of pushConstant: nil's (0 to N)" then there's a bug in the IR somewhere when the number of temps is > 3.�� But if that's not what it means then, well, I don't know what the problem is.

��

For some reason, bytecodeOffset is the *last* byte associated with IR.

This makes little sense to me, the main use seems to be to get correct offsets for different instructions in an IR sequence.

Since block IR includes temp initialization bytes, the offset is 5.

Changing the code to record *starting* offset of an IRNode in IRSequence, by:

mapBytesTo: instr

"Associate the current byte offset with instr. We fix this later to have the correct offset,

see #bytecodes"

instrMap add: instr -> (bytes size + 1)

and mapping bytes *before* visiting node in the IRTranslatorV2:

visitInstruction: instr

gen mapBytesTo: instr.

self visitNode: instr.

and the same in��visitPushClosureCopy: closure

closure copiedValues do: [:name |��

gen mapBytesTo: closure.

gen pushTemp: (self currentScope indexForVarNamed: name).

].

--snip--

Hupps, the pushing of copiedValues are not part of the closure bytecode, so that needs to be��

closure copiedValues do: [:name |

gen pushTemp: (self currentScope indexForVarNamed: name).

].

gen mapBytesTo: closure.

Thanks��testBlockAndContextSourceNode!

After a recompile of the test methods in case of cached IR (and offsets)

(RPackageOrganizer default packageNamed: 'OpalCompiler-Tests') classes do:#recompile

All (existing) tests are green with the attached change set (where haltOnce's were removed too),��at least in the almost-release version of Pharo4 I tested in.

Cheers,

Henry