8390813: C2: Wrong entries in adlc commutative op lists - #32476
8390813: C2: Wrong entries in adlc commutative op lists#32476DingliZhang wants to merge 2 commits into
Conversation
|
👋 Welcome back dzhang! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@DingliZhang The following label will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command. |
|
The total number of required reviews for this PR has been set to 2 based on the presence of this label: |
Webrevs
|
|
It would be really nice if the node types or maybe the rules themselves knew if they were commutative or not, instead of having to do string compares, but I guess that's out of the scope of this fix. |
merykitty
left a comment
There was a problem hiding this comment.
Looks good to me, I have submitted the test.
Hi, please consider.
MatchNode::count_commutative_op()insrc/hotspot/share/adlc/formssel.cppholds two listsof ideal node names that adlc treats as commutative. Neither is correct.
The scalar list is missing a comma:
String literal concatenation folds
"XorL" "UMax"into"XorLUMax", so the list really is{..., "XorI", "XorLUMax", "UMin"}. As entries are compared with strcmp(), XorL is nolonger recognized and adlc stops generating its operand-swapped match rules.
There are also no scalar UMax/UMin ideal nodes; classes.hpp only defines UMinV/UMaxV.
So the same two names in commut_vector_op_list never match either.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/32476/head:pull/32476$ git checkout pull/32476Update a local copy of the PR:
$ git checkout pull/32476$ git pull https://git.openjdk.org/jdk.git pull/32476/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 32476View PR using the GUI difftool:
$ git pr show -t 32476Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/32476.diff
Using Webrev
Link to Webrev Comment