RFR: 8256438: AArch64: Implement match rules with ROR shift register value [v3]
Eric Liu
github.com+10482586+therealeliu at openjdk.java.net
Mon Feb 22 10:33:42 UTC 2021
On Mon, 22 Feb 2021 09:05:53 GMT, Roland Westrelin <roland at openjdk.org> wrote:
>> Eric Liu has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Add benchmark test
>>
>> Change-Id: I63ca51d06070a07e5c20daf4b42d2c8d7237a1da
>
> src/hotspot/share/opto/addnode.cpp line 349:
>
>> 347: }
>> 348: }
>> 349:
>
> Even though existing code in that method seems to assume node's inputs can't be NULL, it's a good practice to protect against unexpected NULLs as that can happen when sub-graphs die during IGVN. So in(1)->in(1), in(1)->in(2), in(2)->in(2) need to be tested for NULL.
>
> That logic and the one for AddLNode are almost identical. So it would be good to have it in a shared method. I've been adding helper methods to make that possible but not all of that code is in yet. Could you file a bug to revisit this issue later and assign it to me?
Hi Roland,
Thanks for your feedback.
> Even though existing code in that method seems to assume node's inputs can't be NULL, it's a good practice to protect against unexpected NULLs as that can happen when sub-graphs die during IGVN. So in(1)->in(1), in(1)->in(2), in(2)->in(2) need to be tested for NULL.
Agree
> That logic and the one for AddLNode are almost identical. So it would be good to have it in a shared method. I've been adding helper methods to make that possible but not all of that code is in yet.
As this match rule is trivial enough, how about withdrawing these shared code in this PR for integrating backend first if your helper methods coming soon?
> Could you file a bug to revisit this issue later and assign it to me?
Okay, it's on my queue now:P
-- Eric
-------------
PR: https://git.openjdk.java.net/jdk/pull/1858
More information about the hotspot-compiler-dev
mailing list