In reviewing #4578 it was discovered that there is a vselect opcode already and hence renaming vternary to it obviously won't work. This leads to the question of what the semantics of both of those opcodes are expected to be.
My understanding from grokking the code is that vternary "selects" one whole vector register over another based on the result of a conditional. vselect maps to a "vector select" instruction implemented on Power and Z that does a bitwise selection from one operand or another under the guidance of a bit mask.
For vternary, only Z provides an implementation in its ternaryEvaluator [1]. However, I'm not convinced it is doing the correct (or expected) thing for vector registers as I can see paths through there that generate instructions that I'm not sure if they apply to vector registers (e.g., SELR).
Here is what I propose:
1) Rename the current IL opcode vselect to vbitselect to have it more closely match the expected semantics on P and Z. This opcode isn't generated in OMR, nor is it generated in any downstream projects that I know of, so this should have zero impact. I'm tempted to suggest just deleting it, but opcode cleanup is a bigger cleanup that I don't think should be done one opcode at a time.
2) Rename vternary to vselect as part of #4578. There are no generators of this opcode in OMR or known downstream projects. This is mainly to maintain symmetry with the other select family opcodes.
3) Mark the new vselect opcode unimplemented in the Z tree evaluator table, unless I have confirmation that the current ternaryEvaluator does what is expected for vector registers.
Opinions @fjeremic @gita-omr ?
I agree with all 3 steps proposed. Looking at the evaluator for the current vternary it is certainly not doing what one would expect. We are likely getting away with this because there are no current uses of that IL.
In reviewing #4581 it was discovered that there is a
vselectopcode already and hence ...
... snip ...
- Rename
vternarytovselectas part of #4581. There are no generators of this ...
I believe in the two instances above the PR that should be being referenced is: #4578 instead of #4581.
Thanks @fjeremic. Unless @gita-omr has objections, @alvee-unb please implement this as part of #4578.
I am fine with the change.
I have done the requested changes along with the changes mentioned in the issue #681 and would appreciate the review. Thank you.
Done. Closing.
Most helpful comment
I believe in the two instances above the PR that should be being referenced is: #4578 instead of #4581.