RFR: 8308444: LoadStoreNode::result_not_used() is too conservative

[Vladimir Kozlov]

On Fri, 19 May 2023 16:19:42 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

> Hi,
> 
> This patch improves the implementation of `LoadStoreNode::result_not_used()` to be less conservative and verifies that the preferable node is matched for `getAndAdd`. Please kindly review, thanks a lot.

Implementation looks good to me. I have few comments about test.
You need second review.

src/hotspot/share/opto/memnode.cpp line 3016:

> 3014: }
> 3015: 
> 3016: bool LoadStoreNode::result_not_used() const {

Add comment to this method explaining what it is for and cases when it returns `true` or `false`.

test/hotspot/jtreg/compiler/c2/x86/TestGetAndAdd.java line 1:

> 1: /*

Please, use `c2/varhandle` directory for this test. We don't use platform in dir names.
The test could be updated later for other platforms.

test/hotspot/jtreg/compiler/c2/x86/TestGetAndAdd.java line 34:

> 32:  * bug 8308444
> 33:  * @summary verify that the correct node is matched for atomic getAndAdd
> 34:  * @requires os.arch=="amd64" | os.arch=="x86_64"

You can use os.simpleArch == "x64" to cover both cases.
And put second `requires` after it so they stay together.

-------------

PR Review: https://git.openjdk.org/jdk/pull/14061#pullrequestreview-1435280691
PR Review Comment: https://git.openjdk.org/jdk/pull/14061#discussion_r1199508347
PR Review Comment: https://git.openjdk.org/jdk/pull/14061#discussion_r1199507955
PR Review Comment: https://git.openjdk.org/jdk/pull/14061#discussion_r1199509039