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

Rework implementations of BINARY_IDENTITY_OP, et al. #7166

Closed
hzongaro opened this issue Nov 1, 2023 · 2 comments
Closed

Rework implementations of BINARY_IDENTITY_OP, et al. #7166

hzongaro opened this issue Nov 1, 2023 · 2 comments
Assignees
@jmesyou
Labels
comp:compiler help wanted tech debt

Comments

@hzongaro
Copy link
Member

hzongaro commented Nov 1, 2023

The BINARY_IDENTITY_OP, BINARY_IDENTITY_OR_ZERO_OP and SIMPLIFY_BRANCH_ARITHMETIC macros in OMRSimplifierHandlers.cpp and OMRSimplifierHelpers.hpp are somewhat complicated, rely on the naming of variables from the context in which they are referenced, and in two cases, cause an early return from the methods in which they are used.

Reworking them to avoid using macros would make them easier to understand and maintain.
@jmesyou jmesyou self-assigned this Nov 15, 2023
@jmesyou
Copy link
Contributor

jmesyou commented Nov 15, 2023

I'll give this one a run and see it how it goes

jmesyou added a commit to jmesyou/omr that referenced this issue Dec 5, 2023
@jmesyou
Refactor Binary Identity Simplification Macro
570ced5 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents that use TR::DataTypes to
signal which type to use for operation.

Fixes: eclipse#7166

Signed-off-by: James You <[email protected]>
@jmesyou jmesyou mentioned this issue Dec 5, 2023
Merged
jmesyou added a commit to jmesyou/omr that referenced this issue Dec 8, 2023
@jmesyou
Refactor Binary Identity Simplification Macro
4060f40 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents that use TR::DataTypes to
signal which type to use for operation.

Fixes: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Dec 10, 2023
@jmesyou
Refactor Binary Identity Simplification Macro
a0a5c5f 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents that use TR::DataTypes to
signal which type to use for operation.

Fixes: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Dec 11, 2023
@jmesyou
Refactor Binary Identity Simplification Macro
03eba01 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents that use TR::DataTypes to
signal which type to use for operation.

Fixes: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Dec 11, 2023
@jmesyou
Refactor Binary Identity Simplification Macro
e83505f 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents that use TR::DataTypes to
signal which type to use for operation.

Fixes: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Dec 11, 2023
@jmesyou
Refactor Binary Identity Simplification Macro
363011f 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents that use TR::DataTypes to
signal which type to use for operation.

Fixes: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Dec 11, 2023
@jmesyou
Refactor Binary Identity Simplification Macro
cccf5d6 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents that use TR::DataTypes to
signal which type to use for operation.

Fixes: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Dec 11, 2023
@jmesyou
Refactor Binary Identity Simplification Macro
273bf22 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents that use TR::DataTypes to
signal which type to use for operation.

Fixes: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Dec 11, 2023
@jmesyou
Refactor Binary Identity Simplification Macro
a3685d9 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents that use TR::DataTypes to
signal which type to use for operation.

Fixes: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Dec 11, 2023
@jmesyou
Refactor Binary Identity Simplification Macro
2c1fda2 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents that use TR::DataTypes to
signal which type to use for operation.

Fixes: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Dec 12, 2023
@jmesyou
Refactor Binary Identity Simplification Macro
f4e05e8 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents that use TR::DataTypes to
signal which type to use for operation.

Fixes: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Dec 19, 2023
@jmesyou
Refactor Binary Identity Simplification Macro
8e6c793 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents that use TR::DataTypes to
signal which type to use for operation.

Fixes: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Feb 5, 2024
@jmesyou
Refactor Binary Identity Simplification Macro
0664dc2 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents by implementing a struct to replace template
instantions of the functions tryAndSimplifyBinaryOp*.

This change limits the number of template instantiations
potential call sites have to create as well as abstracts
some captured variables inside the struct to minimize
the number of arguments passed to actual calls.

A closure is emulated by the struct.
Through escape analysis, scalar replacement and inlining,
the overhead of creating this struct should be minimal
on -O3

Related: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Feb 5, 2024
@jmesyou
Refactor Binary Identity Simplification Macro
6795c73 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents by implementing a struct to replace template
instantions of the functions tryAndSimplifyBinaryOp*.

This change limits the number of template instantiations
potential call sites have to create as well as abstracts
some captured variables inside the struct to minimize
the number of arguments passed to actual calls.

A closure is emulated by the struct.
Through escape analysis, scalar replacement and inlining,
the overhead of creating this struct should be minimal
on -O3

Related: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Feb 14, 2024
@jmesyou
Refactor Binary Identity Simplification Macro
05f401e 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents by implementing a struct to replace template
instantions of the functions tryAndSimplifyBinaryOp*.

This change limits the number of template instantiations
potential call sites have to create as well as abstracts
some captured variables inside the struct to minimize
the number of arguments passed to actual calls.

A closure is emulated by the struct.
Through escape analysis, scalar replacement and inlining,
the overhead of creating this struct should be minimal
on -O3

Related: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Feb 26, 2024
@jmesyou
Refactor Binary Identity Simplification Macro
c529cc0 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents by implementing a struct to replace template
instantions of the functions tryAndSimplifyBinaryOp*.

This change limits the number of template instantiations
potential call sites have to create as well as abstracts
some captured variables inside the struct to minimize
the number of arguments passed to actual calls.

A closure is emulated by the struct.
Through escape analysis, scalar replacement and inlining,
the overhead of creating this struct should be minimal
on -O3

Related: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Mar 5, 2024
@jmesyou
Refactor Binary Identity Simplification Macro
2f17b62 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents by implementing a struct to replace template
instantions of the functions tryAndSimplifyBinaryOp*.

This change limits the number of template instantiations
potential call sites have to create as well as abstracts
some captured variables inside the struct to minimize
the number of arguments passed to actual calls.

A closure is emulated by the struct.
Through escape analysis, scalar replacement and inlining,
the overhead of creating this struct should be minimal
on -O3

Related: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Mar 6, 2024
@jmesyou
Refactor Binary Identity Simplification Macro
217a709 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents by implementing a struct to replace template
instantions of the functions tryAndSimplifyBinaryOp*.

This change limits the number of template instantiations
potential call sites have to create as well as abstracts
some captured variables inside the struct to minimize
the number of arguments passed to actual calls.

A closure is emulated by the struct.
Through escape analysis, scalar replacement and inlining,
the overhead of creating this struct should be minimal
on -O3

Related: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Mar 20, 2024
@jmesyou
Refactor Binary Identity Simplification Macro
0eb4905 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents by implementing a struct to replace template
instantions of the functions tryAndSimplifyBinaryOp*.

This change limits the number of template instantiations
potential call sites have to create as well as abstracts
some captured variables inside the struct to minimize
the number of arguments passed to actual calls.

A closure is emulated by the struct.
Through escape analysis, scalar replacement and inlining,
the overhead of creating this struct should be minimal
on -O3

Related: eclipse#7166

Signed-off-by: James You <[email protected]>
jmesyou added a commit to jmesyou/omr that referenced this issue Mar 21, 2024
@jmesyou
Refactor Binary Identity Simplification Macro
5030f48 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents by implementing a struct to replace template
instantions of the functions tryAndSimplifyBinaryOp*.

This change limits the number of template instantiations
potential call sites have to create as well as abstracts
some captured variables inside the struct to minimize
the number of arguments passed to actual calls.

A closure is emulated by the struct.
Through escape analysis, scalar replacement and inlining,
the overhead of creating this struct should be minimal
on -O3

Related: eclipse#7166

Signed-off-by: James You <[email protected]>
hzongaro pushed a commit to hzongaro/omr that referenced this issue Mar 26, 2024
@jmesyou @hzongaro
Refactor Binary Identity Simplification Macro
c8224d2 
This commit cleans up the BINARY_IDENTITY_OP and
BIANRY_IDENTITY_OR_ZERO macros and replaces them
with inline equivalents by implementing a struct to replace template
instantions of the functions tryAndSimplifyBinaryOp*.

This change limits the number of template instantiations
potential call sites have to create as well as abstracts
some captured variables inside the struct to minimize
the number of arguments passed to actual calls.

A closure is emulated by the struct.
Through escape analysis, scalar replacement and inlining,
the overhead of creating this struct should be minimal
on -O3

Related: eclipse#7166

Signed-off-by: James You <[email protected]>
@hzongaro
Copy link
Member Author

Fixed by pull request #7195

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jmesyou jmesyou

Labels
comp:compiler help wanted tech debt
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jmesyou @hzongaro