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

Add vector mask testAny, testAll and conversion opcodes #6690

Merged
merged 2 commits into from
Oct 14, 2022
Merged

Add vector mask testAny, testAll and conversion opcodes #6690

merged 2 commits into from
Oct 14, 2022

Conversation

gita-omr
Copy link
Contributor

@gita-omr gita-omr commented Sep 2, 2022

  • the opcodes return true iff at least one element(testAny) or all
    elements(testALL) of the vecror mask are true
  • both testAny and testAll opcodes have masked and non-masked
    versions, so add mTestAny, mTestAll, mmTestAny, mmTestAll

@gita-omr
Copy link
Contributor Author

gita-omr commented Sep 6, 2022

Jenkins build all

BradleyWood
BradleyWood reviewed Sep 7, 2022
View reviewed changes
compiler/il/VectorOperations.enum
/* .name = */ "mmAnyTrue", \
/* .properties1 = */ 0, \
/* .properties2 = */ ILProp2::ValueNumberShare | ILProp2::SupportedForPRE, \
/* .properties3 = */ 0, \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need a flag to indicate that this operation is on a mask, under a mask

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree, will add.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added ILProp1::VectorMasked property. But after some thought, not sure about "on mask" property. Usually we check the return type of the opcode vs the type of any of its children. Do you think we need that flag? @BradleyWood

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, its not necessary. I leave it up to you

compiler/il/VectorOperations.enum
/* .name = */ "mAnyTrue", \
/* .properties1 = */ 0, \
/* .properties2 = */ ILProp2::ValueNumberShare | ILProp2::SupportedForPRE, \
/* .properties3 = */ 0, \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a flag to indicate these operations are on a mask (not under a mask)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above. Will keep it like that for now.

@gita-omr
Copy link
Contributor Author

Rebased.

@gita-omr
Copy link
Contributor Author

gita-omr commented Sep 15, 2022 •
edited
Loading

Added VectorMasked property to mmAnyTrue and mmAllTrue.

@knn-k
Copy link
Contributor

knn-k commented Sep 15, 2022

The aarch64/arm part looks good to me.

@gita-omr
Copy link
Contributor Author

Rebased

@gita-omr
Copy link
Contributor Author

Added commit for mask conversion opcodes.

@gita-omr
Copy link
Contributor Author

Fixed line endings

@gita-omr gita-omr changed the title Add vector mask testAny and testAll opcodes Add vector mask testAny, testAll and conversion opcodes Sep 24, 2022
@gita-omr gita-omr mentioned this pull request Sep 24, 2022
Merged
@gita-omr
Copy link
Contributor Author

Jenkins build all

@gita-omr gita-omr changed the title Add vector mask testAny, testAll and conversion opcodes WIP: Add vector mask testAny, testAll and conversion opcodes Sep 26, 2022
@gita-omr
Copy link
Contributor Author

WIP since I would like to add some fixes to this PR.

@gita-omr
Copy link
Contributor Author

Rebased.

@gita-omr
Copy link
Contributor Author

gita-omr commented Oct 3, 2022

Fixed a bug in getVectorOperation().

@gita-omr
Add vector mask testAny and testAll opcodes
9039b0d 
- the opcodes return true iff at least one element(testAny) or all
  elements(testALL) of the vecror mask are true
- both testAny and testAll opcodes have masked and non-masked
  versions, so add mTestAny, mTestAll, mmTestAny, mmTestAll
@gita-omr
Add mask conversion opcodes
7c4a4b9
@gita-omr gita-omr changed the title WIP: Add vector mask testAny, testAll and conversion opcodes Add vector mask testAny, testAll and conversion opcodes Oct 11, 2022
@knn-k
Copy link
Contributor

knn-k commented Oct 14, 2022

Jenkins build all

@knn-k
Copy link
Contributor

knn-k commented Oct 14, 2022

The following failure on linux ppc needs to be investigated in Issue #6571.

[2022-10-14T04:21:57.059Z] 30: [0;32m[----------] [m14 tests from Special/PPCDirectEncodingTest
[2022-10-14T04:21:57.059Z] 30: free(): invalid next size (normal)
[2022-10-14T04:21:57.059Z] 30/30 Test #30: compunittest ......................Child aborted***Exception:   0.75 sec

@knn-k knn-k merged commit d5d4a53 into eclipse-omr:master Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BradleyWood BradleyWood BradleyWood left review comments

@knn-k knn-k knn-k approved these changes

@fjeremic fjeremic Awaiting requested review from fjeremic

@aviansie-ben aviansie-ben Awaiting requested review from aviansie-ben

@0xdaryl 0xdaryl Awaiting requested review from 0xdaryl 0xdaryl is a code owner

@vijaysun-omr vijaysun-omr Awaiting requested review from vijaysun-omr vijaysun-omr is a code owner

@Leonardo2718 Leonardo2718 Awaiting requested review from Leonardo2718

@mstoodle mstoodle Awaiting requested review from mstoodle mstoodle is a code owner

Assignees
No one assigned
Labels
arch:aarch32 arch:aarch64 arch:power arch:riscv arch:x86 arch:z comp:compiler
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gita-omr @knn-k @BradleyWood