Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

vternary and vselect opcodes #4590

Closed
0xdaryl opened this issue Nov 22, 2019 · 6 comments
Closed

vternary and vselect opcodes #4590

0xdaryl opened this issue Nov 22, 2019 · 6 comments
Labels
comp:compiler discussion

Comments

@0xdaryl
Copy link
Contributor

0xdaryl commented Nov 22, 2019 •
edited
Loading

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 Refactoring of TR::ternary opcodes to TR::select #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 ?

[1]

omr/compiler/z/codegen/ControlFlowEvaluator.cpp

Line 2985 in f0489cc

OMR::Z::TreeEvaluator::ternaryEvaluator(TR::Node *node, TR::CodeGenerator *cg)

@fjeremic
Copy link
Contributor

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.

@aarongraham9
Copy link
Contributor

In reviewing #4581 it was discovered that there is a vselect opcode already and hence ...
... snip ...
2. Rename vternary to vselect as 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.

@0xdaryl
Copy link
Contributor Author

0xdaryl commented Nov 22, 2019

Thanks @fjeremic. Unless @gita-omr has objections, @alvee-unb please implement this as part of #4578.

@gita-omr
Copy link
Contributor

I am fine with the change.

alvee-unb added a commit to CAS-Atlantic/omr that referenced this issue Nov 25, 2019
@alvee-unb
Refactoring of TR::ternary opcodes to TR::select
c90236d 
Replacing of the ternary OpCodes such as TR::bternary etc. to
TR::bselect etc. and the associated items respectively.

The changes are made onto the followings:
- Components of various data types such as- Int8, Float, Double
  (iternary, bternary, vternary etc) etc..
- Corresponding evaluators such as- bselectEvaluator, iselectEvaluator
  etc. and other functions where the IL OpCodes are used.
- Corresponding outputs of texts such as- "iselect" etc. and comments
  (//ternary).
- The IL opcode (and its respective evaluator to- vbitselectEvaluator)-
vselect has been updated to vbitselect.
- The new vselect opcode has been marked- unimplemented in the Z tree
evaluator.

Closes: GitHub issue eclipse#681 and eclipse#4590

Signed-off-by: Md. Alvee Noor <mnoor@unb.ca>
alvee-unb added a commit to CAS-Atlantic/omr that referenced this issue Nov 25, 2019
@alvee-unb
Refactoring of TR::ternary opcodes to TR::select
9ca7659 
Replacing of the ternary OpCodes such as TR::bternary etc. to
TR::bselect etc. and the associated items respectively.

The changes are made onto the followings:
- Components of various data types such as- Int8, Float, Double
  (iternary, bternary, vternary etc) etc..
- Corresponding evaluators such as- bselectEvaluator, iselectEvaluator
  etc. and other functions where the IL OpCodes are used.
- Corresponding outputs of texts such as- "iselect" etc. and comments
  (//ternary).
- The IL opcode (and its respective evaluator to- vbitselectEvaluator)-
vselect has been updated to vbitselect.
- The new vselect opcode has been marked- unimplemented in the Z tree
evaluator.

Closes: GitHub issue eclipse#681 and eclipse#4590

Signed-off-by: Md. Alvee Noor <mnoor@unb.ca>
@alvee-unb
Copy link
Contributor

I have done the requested changes along with the changes mentioned in the issue #681 and would appreciate the review. Thank you.

@0xdaryl 0xdaryl mentioned this issue Nov 26, 2019
Merged
alvee-unb added a commit to CAS-Atlantic/omr that referenced this issue Dec 16, 2019
@alvee-unb
Refactoring of TR::ternary opcodes to TR::select
e8f48d1 
Replacing of the ternary OpCodes such as TR::bternary etc. to
TR::bselect etc. and the associated items respectively.

The changes are made onto the followings:
- Components of various data types such as- Int8, Float, Double
  (iternary, bternary, vternary etc) etc..
- Corresponding evaluators such as- bselectEvaluator, iselectEvaluator
  etc. and other functions where the IL OpCodes are used.
- Corresponding outputs of texts such as- "iselect" etc. and comments
  (//ternary).
- The IL opcode (and its respective evaluator to- vbitselectEvaluator)-
vselect has been updated to vbitselect.
- The new vselect opcode has been marked- unimplemented in the Z tree
evaluator.
- File names which involved updating the class names.

Partially Resolves: GitHub issues eclipse#681 and eclipse#4590

Signed-off-by: Md. Alvee Noor <mnoor@unb.ca>
alvee-unb added a commit to CAS-Atlantic/omr that referenced this issue Dec 16, 2019
@alvee-unb
Refactoring of TR::ternary opcodes to TR::select
3948cd5 
Replacing of the ternary OpCodes such as TR::bternary etc. to
TR::bselect etc. and the associated items respectively.

The changes are made onto the followings:
- Components of various data types such as- Int8, Float, Double
  (iternary, bternary, vternary etc) etc..
- Corresponding evaluators such as- bselectEvaluator, iselectEvaluator
  etc. and other functions where the IL OpCodes are used.
- Corresponding outputs of texts such as- "iselect" etc. and comments
  (//ternary).
- The IL opcode (and its respective evaluator to- vbitselectEvaluator)-
vselect has been updated to vbitselect.
- The new vselect opcode has been marked- unimplemented in the Z tree
evaluator.
- File names which involved updating the class names.

Partially Resolves: GitHub issues eclipse#681 and eclipse#4590

Signed-off-by: Md. Alvee Noor <mnoor@unb.ca>
alvee-unb added a commit to CAS-Atlantic/omr that referenced this issue Dec 20, 2019
@alvee-unb
Refactoring of TR::ternary opcodes to TR::select
097199d 
Replacing of the ternary OpCodes such as TR::bternary etc. to
TR::bselect etc. and the associated items respectively.

The changes are made onto the followings:
- Components of various data types such as- Int8, Float, Double
  (iternary, bternary, vternary etc) etc..
- Corresponding evaluators such as- bselectEvaluator, iselectEvaluator
  etc. and other functions where the IL OpCodes are used.
- Corresponding outputs of texts such as- "iselect" etc. and comments
  (//ternary).
- The IL opcode (and its respective evaluator to- vbitselectEvaluator)-
vselect has been updated to vbitselect.
- The new vselect opcode has been marked- unimplemented in the Z tree
evaluator.
- File names which involved updating of the class names.

Partially Resolves: GitHub issues eclipse#681 and eclipse#4590

Signed-off-by: Md. Alvee Noor <mnoor@unb.ca>
alvee-unb added a commit to CAS-Atlantic/omr that referenced this issue Jan 14, 2020
@alvee-unb
Refactoring of TR::ternary opcodes to TR::select
4d02352 
Replacing of the ternary OpCodes such as TR::bternary etc. to
TR::bselect etc. and the associated items respectively.

The changes are made onto the followings:
- Components of various data types such as- Int8, Float, Double
  (iternary, bternary, vternary etc) etc..
- Corresponding evaluators such as- bselectEvaluator, iselectEvaluator
  etc. and other functions where the IL OpCodes are used.
- Corresponding outputs of texts such as- "iselect" etc. and comments
  (//ternary).
- The IL opcode (and its respective evaluator to- vbitselectEvaluator)-
vselect has been updated to vbitselect.
- The new vselect opcode has been marked- unimplemented in the Z tree
evaluator.
- File names which involved updating of the class names.

Partially Resolves: GitHub issues eclipse#681 and eclipse#4590

Signed-off-by: Md. Alvee Noor <mnoor@unb.ca>
alvee-unb added a commit to CAS-Atlantic/omr that referenced this issue Jan 14, 2020
@alvee-unb
Refactoring of TR::ternary opcodes to TR::select
1ad446f 
Replacing of the ternary OpCodes such as TR::bternary etc. to
TR::bselect etc. and the associated items respectively.

The changes are made onto the followings:
- Components of various data types such as- Int8, Float, Double
  (iternary, bternary, vternary etc) etc..
- Corresponding evaluators such as- bselectEvaluator, iselectEvaluator
  etc. and other functions where the IL OpCodes are used.
- Corresponding outputs of texts such as- "iselect" etc. and comments
  (//ternary).
- The IL opcode (and its respective evaluator to- vbitselectEvaluator)-
vselect has been updated to vbitselect.
- The new vselect opcode has been marked- unimplemented in the Z tree
evaluator.
- File names which involved updating of the class names.

Partially Resolves: GitHub issues eclipse#681 and eclipse#4590

Signed-off-by: Md. Alvee Noor <mnoor@unb.ca>
alvee-unb added a commit to CAS-Atlantic/omr that referenced this issue Feb 12, 2020
@alvee-unb
Refactoring of TR::ternary opcodes to TR::select
62e6925 
Replacing of the ternary OpCodes such as TR::bternary etc. to
TR::bselect etc. and the associated items respectively.

The changes are made onto the followings:
- Components of various data types such as- Int8, Float, Double
  (iternary, bternary, vternary etc) etc..
- Corresponding evaluators such as- bselectEvaluator, iselectEvaluator
  etc. and other functions where the IL OpCodes are used.
- Corresponding outputs of texts such as- "iselect" etc. and comments
  (//ternary).
- The IL opcode (and its respective evaluator to- vbitselectEvaluator)-
vselect has been updated to vbitselect.
- The new vselect opcode has been marked- unimplemented in the Z tree
evaluator.
- File names which involved updating of the class names.
- There are some duplicated usages of the opcodes (`aternary`) and
their respective functions for their dependencies on OpenJ9, they will
need to be deleted once the eclipse/openj9 #7797 PR is accepted.

Partially Resolves: GitHub issues eclipse#681 and eclipse#4590

Signed-off-by: Md. Alvee Noor <mnoor@unb.ca>
alvee-unb added a commit to CAS-Atlantic/omr that referenced this issue Feb 19, 2020
@alvee-unb
Refactoring of TR::ternary opcodes to TR::select
e10c9e0 
Replacing of the ternary OpCodes such as TR::bternary etc. to
TR::bselect etc. and the associated items respectively.

The changes are made onto the followings:
- Components of various data types such as- Int8, Float, Double
  (iternary, bternary, vternary etc) etc..
- Corresponding evaluators such as- bselectEvaluator, iselectEvaluator
  etc. and other functions where the IL OpCodes are used.
- Corresponding outputs of texts such as- "iselect" etc. and comments
  (//ternary).
- The IL opcode (and its respective evaluator to- vbitselectEvaluator)-
vselect has been updated to vbitselect.
- The new vselect opcode has been marked- unimplemented in the Z tree
evaluator.
- File names which involved updating of the class names.
- There are some duplicated usages of the opcodes (`aternary`) and
their respective functions for their dependencies on OpenJ9, they will
need to be deleted once the eclipse/openj9 #7797 PR is accepted.

Partially Resolves: GitHub issues eclipse#681 and eclipse#4590

Signed-off-by: Md. Alvee Noor <mnoor@unb.ca>
alvee-unb added a commit to CAS-Atlantic/omr that referenced this issue Feb 25, 2020
@alvee-unb
Refactoring of TR::ternary opcodes to TR::select
66aae96 
Replacing of the ternary OpCodes such as TR::bternary etc. to
TR::bselect etc. and the associated items respectively.

The changes are made onto the followings:
- Components of various data types such as- Int8, Float, Double
  (iternary, bternary, vternary etc) etc..
- Corresponding evaluators such as- bselectEvaluator, iselectEvaluator
  etc. and other functions where the IL OpCodes are used.
- Corresponding outputs of texts such as- "iselect" etc. and comments
  (//ternary).
- The IL opcode (and its respective evaluator to- vbitselectEvaluator)-
vselect has been updated to vbitselect.
- The new vselect opcode has been marked- unimplemented in the Z tree
evaluator.
- File names which involved updating of the class names.
- There are some duplicated usages of the opcodes (`aternary`) and
their respective functions for their dependencies on OpenJ9, they will
need to be deleted once the eclipse/openj9 #7797 PR is accepted.

Partially Resolves: GitHub issues eclipse#681 and eclipse#4590

Signed-off-by: Md. Alvee Noor <mnoor@unb.ca>
alvee-unb added a commit to CAS-Atlantic/omr that referenced this issue Feb 25, 2020
@alvee-unb
Refactoring of TR::ternary opcodes to TR::select
4eea71b 
Replacing of the ternary OpCodes such as TR::bternary etc. to
TR::bselect etc. and the associated items respectively.

The changes are made onto the followings:
- Components of various data types such as- Int8, Float, Double
  (iternary, bternary, vternary etc) etc..
- Corresponding evaluators such as- bselectEvaluator, iselectEvaluator
  etc. and other functions where the IL OpCodes are used.
- Corresponding outputs of texts such as- "iselect" etc. and comments
  (//ternary).
- The IL opcode (and its respective evaluator to- vbitselectEvaluator)-
vselect has been updated to vbitselect.
- The new vselect opcode has been marked- unimplemented in the Z tree
evaluator.
- File names which involved updating of the class names.
- There are some duplicated usages of the opcodes (`aternary`) and
their respective functions for their dependencies on OpenJ9, they will
need to be deleted once the eclipse/openj9 #7797 PR is accepted.

Partially Resolves: GitHub issues eclipse#681 and eclipse#4590

Signed-off-by: Md. Alvee Noor <mnoor@unb.ca>
AlenBadel pushed a commit to AlenBadel/omr that referenced this issue Feb 29, 2020
@alvee-unb
Refactoring of TR::ternary opcodes to TR::select
6af06d1 
Replacing of the ternary OpCodes such as TR::bternary etc. to
TR::bselect etc. and the associated items respectively.

The changes are made onto the followings:
- Components of various data types such as- Int8, Float, Double
  (iternary, bternary, vternary etc) etc..
- Corresponding evaluators such as- bselectEvaluator, iselectEvaluator
  etc. and other functions where the IL OpCodes are used.
- Corresponding outputs of texts such as- "iselect" etc. and comments
  (//ternary).
- The IL opcode (and its respective evaluator to- vbitselectEvaluator)-
vselect has been updated to vbitselect.
- The new vselect opcode has been marked- unimplemented in the Z tree
evaluator.
- File names which involved updating of the class names.
- There are some duplicated usages of the opcodes (`aternary`) and
their respective functions for their dependencies on OpenJ9, they will
need to be deleted once the eclipse/openj9 #7797 PR is accepted.

Partially Resolves: GitHub issues eclipse#681 and eclipse#4590

Signed-off-by: Md. Alvee Noor <mnoor@unb.ca>
alvee-unb added a commit to CAS-Atlantic/openj9 that referenced this issue Mar 2, 2020
@alvee-unb
Refactoring of TR::ternary opcodes to TR::select
f596aea 
Replaced the ternary OpCodes such as TR::aternary etc. to
TR::aselect etc. respectively.

Related OMR PR: eclipse/omr#4578

Partially Resolves: Issue eclipse/omr#681 and eclipse/omr#4590

Signed-off-by: Md. Alvee Noor <mnoor@unb.ca>
PushkarBettadpur pushed a commit to PushkarBettadpur/omr that referenced this issue Mar 13, 2020
@alvee-unb @PushkarBettadpur
Refactoring of TR::ternary opcodes to TR::select
97e7fbd 
Replacing of the ternary OpCodes such as TR::bternary etc. to
TR::bselect etc. and the associated items respectively.

The changes are made onto the followings:
- Components of various data types such as- Int8, Float, Double
  (iternary, bternary, vternary etc) etc..
- Corresponding evaluators such as- bselectEvaluator, iselectEvaluator
  etc. and other functions where the IL OpCodes are used.
- Corresponding outputs of texts such as- "iselect" etc. and comments
  (//ternary).
- The IL opcode (and its respective evaluator to- vbitselectEvaluator)-
vselect has been updated to vbitselect.
- The new vselect opcode has been marked- unimplemented in the Z tree
evaluator.
- File names which involved updating of the class names.
- There are some duplicated usages of the opcodes (`aternary`) and
their respective functions for their dependencies on OpenJ9, they will
need to be deleted once the eclipse/openj9 #7797 PR is accepted.

Partially Resolves: GitHub issues eclipse#681 and eclipse#4590

Signed-off-by: Md. Alvee Noor <mnoor@unb.ca>
@0xdaryl
Copy link
Contributor Author

0xdaryl commented Apr 1, 2020

Done. Closing.

@0xdaryl 0xdaryl closed this as completed Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
comp:compiler discussion
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@aarongraham9 @fjeremic @0xdaryl @gita-omr @alvee-unb