Skip to content

Ensure simplification of compare op is valid - #7712

Merged
hzongaro merged 1 commit into
eclipse-omr:masterfrom
kevindean12:unsigned-cmp-sel
Apr 8, 2025
Merged

Ensure simplification of compare op is valid#7712
hzongaro merged 1 commit into
eclipse-omr:masterfrom
kevindean12:unsigned-cmp-sel

Conversation

@kevindean12

Copy link
Copy Markdown
Contributor

When the compare op is an unsigned compare, unless the select op's DataType is TR::Address, simplifyISelectCompare() will recreate the compare op with a BadILOp because compareOpCode() will not find a valid comparison op of type TR_cmpNE. This will cause problems in the comparison op simplifier afterward. Guards against this simplification in these cases.

@hzongaro hzongaro self-assigned this Apr 4, 2025
Comment thread compiler/optimizer/OMRSimplifierHandlers.cpp Outdated
Comment thread compiler/optimizer/OMRSimplifierHandlers.cpp Outdated
Removes isUnsignedCompare from the call to TR::ILOpcode::compareOpCode
since the comparison to be made is one for inequality. The signedness
of the integer comparison makes no difference in this case.
This prevents unnecessarily returning a BadILOp from compareOpCode(),
as would previously have been done for most unsigned comparisons.
Also adds a check for a BadILOp to ensure that in all future cases
the compare op is a valid IL op before proceeding with simplification.

@hzongaro hzongaro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. Thanks!

@hzongaro

hzongaro commented Apr 8, 2025

Copy link
Copy Markdown
Contributor

Jenkins build all

@hzongaro

hzongaro commented Apr 8, 2025

Copy link
Copy Markdown
Contributor

risc-v testing has been timing out consistently for the past few weeks, so I won't wait for this test run to complete. All other testing was successful. Merging.

@hzongaro
hzongaro merged commit 5f97077 into eclipse-omr:master Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants