Omr: Rename TR::ternary opcode to TR::select

Created on 18 Jan 2017  路  7Comments  路  Source: eclipse/omr

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.

architecture review OK backlog compiler good first issue help wanted

Most helpful comment

I would vote on rename-- it is too useful to get rid of in my opinion.

TR::select?

All 7 comments

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

  • [ ] opCodesForTernarySelect

~/omr/compiler/il/OMRILOpCodeProperties.hpp

  • [ ] ILProp2::Ternary
  • [ ] TR::vternary

~/omr/compiler/il/OMRNode.cpp

  • [ ] ternarySeenCollected

We can discuss more on the next CAM.

I have done the requested changes and would appreciate the review. Thank you.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

0xdaryl picture 0xdaryl  路  5Comments

0xdaryl picture 0xdaryl  路  4Comments

0xdaryl picture 0xdaryl  路  6Comments

0xdaryl picture 0xdaryl  路  5Comments

sajidahmed21 picture sajidahmed21  路  3Comments