eclipse / omr

Eclipse OMR™ Cross platform components for building reliable, high performance language runtimes
http://www.eclipse.org/omr
Other
943 stars 396 forks source link

Convert TR::tstart opcode to use TR::Case child instead of TR::branch #1847

Open mgaudet opened 6 years ago

mgaudet commented 6 years ago

The tstart opcode is used to represent the start of a transaction in IL

It takes the shape of a node with three branch children today:

n3n       BBStart <block_3>                                                     
n22n      tstart  unknown helper[#180  helper Method] [flags 0x400 0x0 ] ()     
n19n        branch --> block_5 BBStart at n7n                                   
n20n        branch --> block_7 BBStart at n11n                                  
n21n        branch --> block_9 BBStart at n15n                                  
n4n       BBEnd </block_3> =====                                                

However, the implementation of tstart can be best described in pseudo C code as:

switch (start_transaction_and_get_return_code()) { 
   case TransactionStartedSuccessfully: goto success; 
   case TransactionFailedPersistent:    goto persistent_failure; 
   case TransactionFailedTransient:     goto transient_failure;
}

(there is no default branch here, and note, I may have gotten branch order incorrect)

While investigating another issue (#1565) it has become clear that in terms of semantics, it may be best to covert tstart to use Case children, which are already used in table and lookup IL switch opcodes. The Branch opcode has a bit of a murky history, and it seems things have changed since it was first implemented.

This change should be testable with Tril

mgaudet commented 6 years ago

(If we removed tstart's use of branch, does that eliminate its usefulness entirely? Should branch also be removed as part of this?)

mstoodle commented 6 years ago

(If we removed tstart's use of branch, does that eliminate its usefulness entirely? Should branch also be removed as part of this?)

I believe TR::branch is only used by our transactional memory support, so yes would be my vote.