As per the discussion in #668 and the subsequent answer by @andrewcraik (https://github.com/eclipse/omr/pull/668#issuecomment-273605969) it seems the TR::ternary IL opcode is slightly misleading to ignorant developers such as myself.
We should document that the semantics of this IL opcode are that all children will be evaluated and executed, and hence the children should have no side-effects should you try to emulate the canonical meaning of a ternary operator in many languages (:?).
An alternative here would be to rename the opcode all together to avoid the confusion.
I would vote on rename-- it is too useful to get rid of in my opinion.
TR::select?
Given only opinions in favour of renaming, the TR::ternary IL opcode shall be renamed to TR::select. Some documentation describing its behaviour is in order as well.
I am renaming all the variables such as- TR::bternary, TR::sternary etc. to TR::bselect, TR::sselect etc. respectively. Also renaming the other functions those were named depending on these variables such as- bternaryEvaluator etc. to bselectEvaluator etc. including all such comments.
Although this issue predates the Compiler Architecture Meetings and the current standard for community review for new opcodes (or changes to existing opcodes), I would like to tag this one for discussion at the next CAM in case there are any concerns. Prior to that, if anyone has any concerns or comments please bring that up in this PR.
Sounds good @0xdaryl, thanks!
Thanks @0xdaryl , can you please also let me know whether the following changes are needed for the components in the respective files (and more)-
~/omr/compiler/aarch64/codegen/OMRTreeEvaluator.hpp
vternaryEvaluator~/omr/compiler/codegen/CodeGenPrep.cpp
>rematerializeCmpUnderTernary~/omr/compiler/il/OMRIL.cpp
~/omr/compiler/il/OMRILOpCodeProperties.hpp
~/omr/compiler/il/OMRNode.cpp
We can discuss more on the next CAM.
I have done the requested changes and would appreciate the review. Thank you.
Most helpful comment
I would vote on rename-- it is too useful to get rid of in my opinion.
TR::select?