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.

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

Stackmap updates to the withfield and defaultvalue instructions #7239

Merged

Conversation

@XxAdi101xX
Copy link
Contributor

XxAdi101xX commented Sep 25, 2019

This commit does the following:

  • updates the erroraneous values associated with the defaultvalue bytecode instruction
  • adds the missing withfield switchcase in stackmap.c
  • fixes some bugs regarding the withfield instruction in StackMap.java

Signed-off-by: Adithya Venkatarao adi_101@live.com

@XxAdi101xX XxAdi101xX force-pushed the XxAdi101xX:persistant-flattened-valuetype-array-test branch from 5070e42 to 06e1f10 Sep 25, 2019
@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Sep 25, 2019

@tajila ready for review

@XxAdi101xX XxAdi101xX force-pushed the XxAdi101xX:persistant-flattened-valuetype-array-test branch 2 times, most recently from 480d8ae to d9fb44b Sep 26, 2019
@XxAdi101xX XxAdi101xX force-pushed the XxAdi101xX:persistant-flattened-valuetype-array-test branch 2 times, most recently from 121693c to f98542c Oct 7, 2019
@XxAdi101xX XxAdi101xX changed the title Added a test where flattened valuetype arrays are kept in a live set Stackmap updates to the withfield and stackmap instructions Oct 7, 2019
@XxAdi101xX XxAdi101xX force-pushed the XxAdi101xX:persistant-flattened-valuetype-array-test branch from f98542c to fd30e6e Oct 7, 2019
@XxAdi101xX XxAdi101xX changed the title Stackmap updates to the withfield and stackmap instructions Stackmap updates to the withfield and defaultvalue instructions Oct 7, 2019
@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Oct 7, 2019

@tajila only the VM changes have been added to this PR, please review

runtime/util/argcount.c Outdated Show resolved Hide resolved
runtime/util/pcstack.c Outdated Show resolved Hide resolved
runtime/stackmap/stackmap.c Outdated Show resolved Hide resolved
@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Oct 10, 2019

@tajila changes have been made; regarding the unimplemented bytecode instruction comment, as mentioned above, I've compared the code in pcstack.c and PCstack.java for defaultvalue and withfield and they seem to be the same, hence I've not made any additional changes to that

@XxAdi101xX XxAdi101xX force-pushed the XxAdi101xX:persistant-flattened-valuetype-array-test branch 2 times, most recently from eb5ed19 to d83b61d Oct 10, 2019
runtime/util/argcount.c Outdated Show resolved Hide resolved
runtime/util/argcount.c Outdated Show resolved Hide resolved
@tajila

This comment has been minimized.

Copy link
Contributor

tajila commented Oct 10, 2019

changes have been made; regarding the unimplemented bytecode instruction comment, as mentioned above, I've compared the code in pcstack.c and PCstack.java for defaultvalue and withfield and they seem to be the same, hence I've not made any additional changes to that

		0x0 /* JBunimplemented (16rCB) */,
		0x0 /* JBunimplemented (16rCC) */,

Those unimplemented bytecodes do not exist in the VM version

@XxAdi101xX XxAdi101xX force-pushed the XxAdi101xX:persistant-flattened-valuetype-array-test branch 2 times, most recently from dde6d7c to df11826 Oct 11, 2019
@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Oct 11, 2019

@tajila I've updated pcstack.c to mirror PCstack.java addressed the other comments, and added changes to maintain stylistic consistency for arrays

@XxAdi101xX XxAdi101xX force-pushed the XxAdi101xX:persistant-flattened-valuetype-array-test branch from df11826 to 7763449 Oct 11, 2019
@tajila

This comment has been minimized.

Copy link
Contributor

tajila commented Oct 11, 2019

I've updated pcstack.c to mirror PCstack.java addressed the other comments, and added changes to maintain stylistic consistency for arrays

Dont about whitespace differences in pcstack, just the comments and array values

runtime/stackmap/stackmap.c Outdated Show resolved Hide resolved
@XxAdi101xX XxAdi101xX force-pushed the XxAdi101xX:persistant-flattened-valuetype-array-test branch from 4cd71fa to fcff733 Oct 16, 2019
@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Oct 16, 2019

@tajila addressed comment and removed duplicate bytecode that were causing build failures

This commit does the following:
- updates the erroraneous values associated with the defaultvalue bytecode instruction
- adds the missing withfield switchcase in stackmap.c
- fixes some bugs regarding the withfield instruction in StackMap.java

Signed-off-by: Adithya Venkatarao <adi_101@live.com>
@XxAdi101xX XxAdi101xX force-pushed the XxAdi101xX:persistant-flattened-valuetype-array-test branch from 362ac4d to 70169e7 Oct 16, 2019
@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Oct 16, 2019

@tajila reverted whitespace changes, ready for review

@tajila
tajila approved these changes Oct 17, 2019
@tajila

This comment has been minimized.

Copy link
Contributor

tajila commented Oct 17, 2019

@gacholio please review these changes

@gacholio

This comment has been minimized.

Copy link
Contributor

gacholio commented Oct 18, 2019

jenkins test sanity xlinux jdk11

@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Oct 18, 2019

@gacholio please hold off on merging temporarily, my personal builds were failing on a jitcompiler issue so I've rebased on another branch with this commit cherry picked and I'm rerunning the tests on both linux and window, to ensure that my changes aren't breaking

@XxAdi101xX

This comment has been minimized.

Copy link
Contributor Author

XxAdi101xX commented Oct 21, 2019

@gacholio builds are passing, the pr is safe to merge

@gacholio gacholio merged commit bc4b0b1 into eclipse:master Oct 21, 2019
7 checks passed
7 checks passed
Build_JDK11_x86-64_linux_Personal Build PASSED
Details
Copyright Check Copyrights appear to be up to date
Details
Line Endings Check Line endings appear to be correct
Details
Pull Request - OpenJ9 All jobs passed
Details
Test_openjdk11_j9_sanity.functional_x86-64_linux_Personal Build PASSED
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
eclipsefdn/eca The author(s) of the pull request is covered by necessary legal agreements in order to proceed!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.