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

Remove PANAMA code #12603

Closed
wants to merge 5 commits into from
Closed

Remove PANAMA code #12603

wants to merge 5 commits into from

Conversation

ohbus
Copy link

@ohbus ohbus commented Apr 30, 2021

Resolves #10377

The native code paths protected by J9VM_OPT_PANAMA and the Java code protected by PANAMA are stale: that code should be removed along with the configuration options.

@fjeremic fjeremic requested a review from keithc-ca April 30, 2021 18:15
@keithc-ca keithc-ca self-assigned this Apr 30, 2021
@ohbus
Copy link
Author

ohbus commented May 3, 2021

Please let me know what to do with this

<include-if condition="spec.flags.J9VM_OPT_PANAMA"/>

@ohbus ohbus marked this pull request as ready for review May 3, 2021 05:00
@ohbus
Copy link
Author

ohbus commented May 3, 2021

Hi, @keithc-ca I have made the changes. Although I believe there will be changes that need to be made after your review.

Q: How to mitigate the copyright issue?


I have made individual commits so that it is easier to review and make changes accordingly. Once it is reviewed, I can squash the commits to one if you would like it that way. Thanks

@ohbus
Copy link
Author

ohbus commented May 3, 2021

Please let me know of a way to fix the following copyright errors as reported.

10:43:07  ###################################
[Pipeline] echo
10:43:07  The following files were modified and have incorrect copyrights
[Pipeline] echo
10:43:07  runtime/jcl/common/java_dyn_methodhandle.c
[Pipeline] echo
10:43:07  runtime/jcl/common/stdinit.c
[Pipeline] echo
10:43:07  runtime/oti/VM_MethodHandleKinds.h
[Pipeline] echo
10:43:07  runtime/vm/MHInterpreter.hpp
[Pipeline] echo
10:43:07  runtime/vm/OutOfLineINL.hpp
[Pipeline] echo
10:43:07  runtime/vm/module.xml
[Pipeline] echo
10:43:07  ###################################

@keithc-ca
Copy link
Contributor

Please let me know what to do with this

	<include-if condition="spec.flags.J9VM_OPT_PANAMA"/>

Normally, the answer would simply be 'remove that line', but the consequence here would be the enclosing <artifact> would then be unconditionally included. In this case, I think the condition should have been

	<include-if condition="spec.flags.opt_jitserver"/>

That's what it should become now.

@keithc-ca
Copy link
Contributor

Please let me know of a way to fix the following copyright errors as reported.

10:43:07 The following files were modified and have incorrect copyrights
10:43:07 runtime/jcl/common/java_dyn_methodhandle.c

Each source file should have a copyright notice at the beginning with two dates; the first is the year the file was first created, the second is when it was last changed. So, for example, in the case of java_dyn_methodhandle.c

 * Copyright (c) 2001, 2020 IBM Corp. and others

should be changed to

 * Copyright (c) 2001, 2021 IBM Corp. and others

Please follow the same rule for every file you change (if it already says 2021, you have nothing to do).

@ohbus
Copy link
Author

ohbus commented May 3, 2021

Hi @keithc-ca, I made the updates to the necessary files.

Thanks for the quick and accurate suggestions.

Please let me know if the PR needs a rebase.

runtime/oti/VM_MethodHandleKinds.h Outdated Show resolved Hide resolved
runtime/vm/MHInterpreter.inc Outdated Show resolved Hide resolved
runtime/vm/module.xml Outdated Show resolved Hide resolved
@ohbus
Copy link
Author

ohbus commented May 6, 2021

There are still a lot of traces which has the word panama in it.

buildspecs/aix_ppc-64.spec:233:		<flag id="opt_panama" value="false"/>
buildspecs/aix_ppc-64_cmprssptrs.spec:231:		<flag id="opt_panama" value="false"/>
buildspecs/aix_ppc.spec:230:		<flag id="opt_panama" value="false"/>
buildspecs/j9.flags:665:	<flag id="graph_enableTesting_Panama">
buildspecs/j9.flags:1828:	<flag id="opt_panama">
buildspecs/j9.flags:1829:		<description>Enables support for Project Panama features such as native method handles</description>
buildspecs/j9.flags:1830:		<ifRemoved>No support for Project Panama features such as native method handles</ifRemoved>
buildspecs/linux_390-64.spec:231:		<flag id="opt_panama" value="false"/>
buildspecs/linux_390-64_cmprssptrs.spec:235:		<flag id="opt_panama" value="false"/>
buildspecs/linux_390.spec:232:		<flag id="opt_panama" value="false"/>
buildspecs/linux_aarch64.spec:212:		<flag id="opt_panama" value="false"/>
buildspecs/linux_aarch64_cmprssptrs.spec:213:		<flag id="opt_panama" value="false"/>
buildspecs/linux_aarch64_cmprssptrs_cross.spec:214:		<flag id="opt_panama" value="false"/>
buildspecs/linux_aarch64_cross.spec:213:		<flag id="opt_panama" value="false"/>
buildspecs/linux_arm.spec:232:		<flag id="opt_panama" value="false"/>
buildspecs/linux_arm_linaro.spec:231:		<flag id="opt_panama" value="false"/>
buildspecs/linux_ppc-64.spec:231:		<flag id="opt_panama" value="false"/>
buildspecs/linux_ppc-64_cmprssptrs.spec:229:		<flag id="opt_panama" value="false"/>
buildspecs/linux_ppc-64_cmprssptrs_le.spec:236:		<flag id="opt_panama" value="false"/>
buildspecs/linux_ppc-64_le.spec:234:		<flag id="opt_panama" value="false"/>
buildspecs/linux_ppc.spec:229:		<flag id="opt_panama" value="false"/>
buildspecs/linux_riscv64.spec:227:		<flag id="opt_panama" value="false"/>
buildspecs/linux_riscv64_cmprssptrs.spec:227:		<flag id="opt_panama" value="false"/>
buildspecs/linux_riscv64_cmprssptrs_cross.spec:228:		<flag id="opt_panama" value="false"/>
buildspecs/linux_riscv64_cross.spec:228:		<flag id="opt_panama" value="false"/>
buildspecs/linux_x86-64.spec:232:		<flag id="opt_panama" value="false"/>
buildspecs/linux_x86-64_cmprssptrs.spec:229:		<flag id="opt_panama" value="false"/>
buildspecs/linux_ztpf_390-64.spec:221:		<flag id="opt_panama" value="false"/>
buildspecs/osx_x86-64.spec:230:		<flag id="opt_panama" value="false"/>
buildspecs/osx_x86-64_cmprssptrs.spec:227:		<flag id="opt_panama" value="false"/>
buildspecs/win_x86-64.spec:237:		<flag id="opt_panama" value="false"/>
buildspecs/win_x86-64_cmprssptrs.spec:234:		<flag id="opt_panama" value="false"/>
buildspecs/win_x86.spec:244:		<flag id="opt_panama" value="false"/>
buildspecs/zos_390-64.spec:232:		<flag id="opt_panama" value="false"/>
buildspecs/zos_390-64_cmprssptrs.spec:232:		<flag id="opt_panama" value="false"/>
buildspecs/zos_390.spec:229:		<flag id="opt_panama" value="false"/>
debugtools/DDR_VM/data/superset-constants.dat:941:C|opt_panama
debugtools/DDR_VM/data/superset-constants.dat:4186:C|JCL_RTFLAG_OPT_PANAMA
debugtools/DDR_VM/generate.properties:42:linux_x86-64_combo,linux_x86-64_cmprssptrs_combo,linux_x86-64_cmprssptrs_panama
debugtools/DDR_VM/generate.properties:50:linux_x86-64,linux_x86-64_cmprssptrs,linux_x86-64_cmprssptrs_panama,\
doc/processes/labels.md:80:* `project:panama`
runtime/gc_glue_java/configure_includes/configure_linux_x86.mk:46:ifeq (linux_x86-64_cmprssptrs_panama, $(SPEC))
runtime/jcl/uma/se9_exports.xml:68:		<include-if condition="spec.flags.opt_panama and spec.flags.opt_methodHandle" />
runtime/oti/vmconstantpool.xml:95:	<classref name="java/lang/invoke/NativeMethodHandle" flags="opt_panama"/>
runtime/oti/vmconstantpool.xml:298:	<fieldref class="java/lang/invoke/NativeMethodHandle" name="J9NativeCalloutDataRef" signature="J" cast="J9NativeCalloutData *" flags="opt_panama"/>
test/docs/OpenJ9TestUserGuide.md:61:    JDK_VERSION=[8|9|10|11|12|Panama|Valhalla] (8 default value, could be auto detected)
test/docs/OpenJ9TestUserGuide.md:121:      - version:  [8|8+|9|9+|10|10+|11|11+|Panama|Valhalla] (filter test based on 
test/functional/Panama/README.md:23:# Panama Test
test/functional/Panama/README.md:24:- Contains [tests](src/org/openj9/test/panama/) for native method handles
test/functional/Panama/README.md:25:- Build with the "Build git tests - panama" job
test/functional/Panama/README.md:26:- Run with linux_x86-64_cmprssptrs_panama.spec on the rawbuild-panama_xa64-b136 sdk
test/functional/Panama/README.md:27:- [HigherLevelNativeMethodHandleTest](src/org/openj9/test/panama/HigherLevelNativeMethodHandleTest.java) uses [panamatest.jar](lib/panamatest.jar), created from [panamatest.c](panamatest.c) with the jextract groveller
test/functional/Panama/README.md:30:    * To create a libpanamatest.so, a gcc is needed:
test/functional/Panama/README.md:32:            gcc -shared -o {$common.workspace.gittest$}/Panama/libpanamatest.so -fPIC {$common.workspace.gittest$}/Panama/panamatest.c
test/functional/Panama/README.md:34:    * To create a file like panamatest.jar, on a machine with os:linux:ubuntu14 capability (such as ub1404x64vm1), run:
test/functional/Panama/README.md:37:            <OpenJDK_root>/bin/jextract -J-Djava.library.path=<path_to_clang_lib>/clang -t panama.test -o panamatest.jar <path to the test repo>/Panama/panamatest.c
test/functional/Panama/build.xml:32:	<property name="DEST" value="${BUILD_ROOT}/functional/Panama" />
test/functional/Panama/build.xml:48:			<fileset dir="${TEST_ROOT}/functional/Panama" includes="libpanamatest.so" />
test/functional/Panama/build.xml:49:			<fileset dir="${LIB_DIR}" includes="panamatest.jar" />
test/functional/Panama/build.xml:66:					<pathelement location="${DEST}/panamatest.jar"/>
test/functional/Panama/build.xml:73:		<jar jarfile="${DEST}/PanamaTests.jar" filesonly="true">
test/functional/Panama/build.xml:85:			<equals arg1="${JDK_VERSION}" arg2="Panama"/>
test/functional/Panama/playlist.xml:27:	-cp $(Q)$(RESOURCES_DIR)$(P)$(TESTNG)$(P)$(TEST_RESROOT)$(D)PanamaTests.jar$(Q) \
test/functional/Panama/playlist.xml:40:			<version>Panama</version>
test/functional/Panama/playlist.xml:46:	-cp $(Q)$(RESOURCES_DIR)$(P)$(TESTNG)$(P)$(TEST_RESROOT)$(D)panamatest.jar$(P)$(TEST_RESROOT)$(D)PanamaTests.jar$(Q) \
test/functional/Panama/playlist.xml:59:			<version>Panama</version>
test/functional/Panama/playlist.xml:65:	-cp $(Q)$(RESOURCES_DIR)$(P)$(TESTNG)$(P)$(TEST_RESROOT)$(D)panamatest.jar$(P)$(TEST_RESROOT)$(D)PanamaTests.jar$(Q) \
test/functional/Panama/playlist.xml:78:			<version>Panama</version>
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:22:package org.openj9.test.panama;
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:33:import panama.test.panamatest;
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:34:import panama.test.panamatest.*;
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:44:	panamatest panama;
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:49:		Library lib = NativeLibrary.loadLibraryFile(path + "/libpanamatest.so");
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:50:		panama = NativeLibrary.bindRaw(panamatest.class, lib);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:56:		Pointer<Integer> arr = panama.getIntPtr();
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:57:		Assert.assertEquals(1, panama.checkIntPtr(arr));
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:58:		panama.freeIntPtr(arr);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:63:		Pointer<Point> sa = panama.getStructArray();
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:64:		Assert.assertEquals(1, panama.checkStructArray(sa));
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:65:		panama.freeStructArray(sa);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:70:		Point p1 = panama.getPoint(2, 3);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:71:		Point p2 = panama.getPoint(4, 6);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:72:		Assert.assertEquals(1, panama.checkPoint(p1, 2, 3));
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:77:		Pointer<Point> pptr = panama.getPointPtr(7, 8);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:78:		Assert.assertEquals(1, panama.checkPointPtr(pptr, 7, 8));
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:79:		panama.freePointPtr(pptr);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:84:		Point p1 = panama.getPoint(2, 3);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:85:		Point p2 = panama.getPoint(4, 6);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:86:		Line ln = panama.getLine(p1, p2);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:87:		Assert.assertEquals(1, panama.checkLine(ln, p1, p2));
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:92:		SmallStruct small = panama.getSmallStruct((byte)4);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:93:		Assert.assertEquals(1, panama.checkSmallStruct(small, (byte)4));
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:98:		Point p1 = panama.getPoint(2, 3);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:99:		Point p2 = panama.getPoint(4, 6);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:100:		Line ln = panama.getLine(p1, p2);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:102:		LargeStruct large = panama.getLargeStruct(str, 4, ln.ptr(), p1, 1, 2, 3, 2.5);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:103:		Assert.assertEquals(1, panama.checkLargeStruct(large, str, 4, ln, p1, 1, 2, 3, 2.5));
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:108:		Padding pad = panama.getPadding(99999999999999999L, 8, 99999999999999999L);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:109:		Assert.assertEquals(1, panama.checkPadding(pad, 99999999999999999L, 8, 99999999999999999L));
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:114:		Complex comp = panama.getComplex(99999999999999999L, 4.6);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:115:		Assert.assertEquals(1, panama.checkComplex(comp, 99999999999999999L, 4.6));
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:120:		Point p1 = panama.getPoint(2, 3);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:121:		ScalarVector sv = panama.getScalarVector(2, p1);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:122:		Assert.assertEquals(1, panama.checkScalarVector(sv, 2, p1));
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:127:		Point p1 = panama.getPoint(2, 3);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:129:		PointerVector pv = panama.getPointerVector(str1, p1);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:130:		Assert.assertEquals(1, panama.checkPointerVector(pv, str1, p1));
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:137:		PointerPointer pp = panama.getPointerPointer(str1, str2);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:138:		Assert.assertEquals(1, panama.checkPointerPointer(pp, str1, str2));
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:143:		Point p1 = panama.getPoint(2, 3);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:144:		Point p2 = panama.getPoint(4, 6);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:145:		Point p3 = panama.getPoint(8, 0);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:146:		Line ln1 = panama.getLine(p1, p2);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:147:		Line ln2 = panama.getLine(p1, p3);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:148:		Line ln3 = panama.getLine(p3, p2);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:149:		ArrayStruct arr = panama.getArrayStruct(ln1, ln2, ln3, 1, 2, 3, 4);
test/functional/Panama/src/org/openj9/test/panama/CustomFFINativeMethodHandleTest.java:150:		Assert.assertEquals(1, panama.checkArrayStruct(arr, ln1, ln2, ln3, 1, 2, 3, 4));
test/functional/Panama/src/org/openj9/test/panama/HigherLevelNativeMethodHandleTest.java:22:package org.openj9.test.panama;
test/functional/Panama/src/org/openj9/test/panama/HigherLevelNativeMethodHandleTest.java:31:import panama.test.panamatest;
test/functional/Panama/src/org/openj9/test/panama/HigherLevelNativeMethodHandleTest.java:41:	panamatest panama;
test/functional/Panama/src/org/openj9/test/panama/HigherLevelNativeMethodHandleTest.java:45:		Library lib = NativeLibrary.loadLibraryFile(path + "/libpanamatest.so");
test/functional/Panama/src/org/openj9/test/panama/HigherLevelNativeMethodHandleTest.java:46:		panama = NativeLibrary.bindRaw(panamatest.class, lib);
test/functional/Panama/src/org/openj9/test/panama/HigherLevelNativeMethodHandleTest.java:51:		byte resultByte = panama.addTwoByte((byte)2,(byte)3);
test/functional/Panama/src/org/openj9/test/panama/HigherLevelNativeMethodHandleTest.java:57:		double resultDouble = panama.addTwoDouble(2.1,3.1);
test/functional/Panama/src/org/openj9/test/panama/HigherLevelNativeMethodHandleTest.java:63:		float resultFloat = panama.addTwoFloat(2.1f,3.1f);
test/functional/Panama/src/org/openj9/test/panama/HigherLevelNativeMethodHandleTest.java:69:		int resultInt = panama.addTwoInt(2,3);
test/functional/Panama/src/org/openj9/test/panama/HigherLevelNativeMethodHandleTest.java:75:		long resultLong = panama.addTwoLong(200000000000000L, 300000000000000L);
test/functional/Panama/src/org/openj9/test/panama/HigherLevelNativeMethodHandleTest.java:81:		short resultShort = panama.addTwoShort((short)2,(short)3);
test/functional/Panama/src/org/openj9/test/panama/HigherLevelNativeMethodHandleTest.java:87:		int resultFour = panama.returnFour();
test/functional/Panama/src/org/openj9/test/panama/HigherLevelNativeMethodHandleTest.java:93:		panama.voidWithArgs(2);
test/functional/Panama/src/org/openj9/test/panama/HigherLevelNativeMethodHandleTest.java:98:		panama.voidNoArgs();
test/functional/Panama/src/org/openj9/test/panama/HigherLevelNativeMethodHandleTest.java:103:		double resultMany = panama.manyArgs(1, 2.5f, 3L, 4.5);
test/functional/Panama/src/org/openj9/test/panama/NativeMethodHandleTest.java:22:package org.openj9.test.panama;
test/functional/Panama/src/org/openj9/test/panama/NativeMethodHandleTest.java:43:		lib = NativeLibrary.loadLibraryFile(path + "/libpanamatest.so");
test/functional/Panama/testng.xml:25:<suite name="Panama suite" parallel="none" verbose="2">
test/functional/Panama/testng.xml:30:		<parameter name="path" value="${JVM_TEST_ROOT}/functional/Panama"/>
test/functional/Panama/testng.xml:32:			<class name="org.openj9.test.panama.NativeMethodHandleTest" />
test/functional/Panama/testng.xml:36:		<parameter name="path" value="${JVM_TEST_ROOT}/functional/Panama"/>
test/functional/Panama/testng.xml:38:			<class name="org.openj9.test.panama.HigherLevelNativeMethodHandleTest" />
test/functional/Panama/testng.xml:42:		<parameter name="path" value="${JVM_TEST_ROOT}/functional/Panama"/>
test/functional/Panama/testng.xml:44:			<class name="org.openj9.test.panama.CustomFFINativeMethodHandleTest" />
test/functional/build.xml:43:			<equals arg1="${JDK_VERSION}" arg2="Panama"/>
test/functional/build.xml:47:						<include name="Panama/build.xml" />
test/functional/build.xml:54:						<exclude name="Panama/build.xml" />

@keithc-ca
Copy link
Contributor

There are still a lot of traces which has the word panama in it.

Yes, that's true. As I described in the issue, those flags need to be removed from j9.flags as well as references from *.spec.

The files under test/functional/Panama may have value relating to jdk.incubator.foreign or inline types. I think we can leave them alone for now.

@ohbus
Copy link
Author

ohbus commented May 16, 2021

Sorry for the delayed response. Lately, I Have been a bit occupied.

I will be submitting the next set of changes by the end of this coming week.

@genie-openj9
Copy link

Can one of the admins verify this patch?

@keithc-ca
Copy link
Contributor

The 'Linter' failure is because runtime/vm/CMakeLists.txt still mentions OutOfLineINL_java_lang_invoke_NativeMethodHandle.cpp which is removed by this.

@ohbus
Copy link
Author

ohbus commented Jun 12, 2021

I truly apologize for the delayed response and submission.

I have made the necessary changes, you can let me know of additional things that needs to be done.

Thank you for the help @keithc-ca

buildspecs/linux_arm_linaro.spec Outdated Show resolved Hide resolved
@@ -65,6 +65,5 @@
<export name="Java_java_lang_StackWalker_walkWrapperImpl" />
<export name="Java_java_lang_StackWalker_getImpl" />
<export name="Java_java_lang_invoke_MethodHandles_findNativeAddress">
<include-if condition="spec.flags.opt_panama and spec.flags.opt_methodHandle" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Only the panama part of the condition should be removed; it needs to be

    <include-if condition="spec.flags.opt_methodHandle" />

Copy link
Author

Choose a reason for hiding this comment

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

I have corrected this condition.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, the export for Java_java_lang_invoke_MethodHandles_findNativeAddress should just be removed (like it is for cmake).

Copy link
Author

Choose a reason for hiding this comment

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

Hi, sorry for the delay.

I have addressed this.

Copy link
Contributor

Choose a reason for hiding this comment

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

The <export> element must be removed as a whole; removing just line 67 yields invalid XML.

runtime/vm/MHInterpreter.inc Show resolved Hide resolved
@keithc-ca
Copy link
Contributor

The copyright check failure forsuperset-constants.dat is to be expected: the file format doesn't permit comments.

@ohbus
Copy link
Author

ohbus commented Jun 14, 2021

Hi @keithc-ca, Thank you so much for the input ⭐ You can re-review now as I have updated the changes as you have requested.

@keithc-ca
Copy link
Contributor

At least two further changes are required due to files removed by this pull request:

  1. runtime/vm/CMakeLists.txt must not refer to OutOfLineINL_java_lang_invoke_NativeMethodHandle.cpp
  2. runtime/vm/MHInterpreter.hpp cannot #include "FFITypeHelpers.hpp"

@ohbus
Copy link
Author

ohbus commented Jun 14, 2021

Done with these two references update.

@keithc-ca
Copy link
Contributor

Jenkins test sanity zlinux,zlinuxuma jdk11

@keithc-ca
Copy link
Contributor

Jenkins test sanity zlinuxuma jdk11

@keithc-ca
Copy link
Contributor

I think the test failure due to -Xnocompressedrefs is unrelated to the changes here.

I think the changes are good, but I'd like to see the number of commits reduced. A single commit works for me, but if you'd like to organize them into a reasonable number of logically related changes, I'd be fine with that too.

@ohbus
Copy link
Author

ohbus commented Jun 16, 2021

I think the test failure due to -Xnocompressedrefs is unrelated to the changes here.

I think the changes are good, but I'd like to see the number of commits reduced. A single commit works for me, but if you'd like to organize them into a reasonable number of logically related changes, I'd be fine with that too.

Let me do that for you for sure.

@ohbus
Copy link
Author

ohbus commented Jun 16, 2021

Done

@keithc-ca
Copy link
Contributor

In collapsing the commits we've arrived in a state that no longer removes these files:

runtime/vm/FFITypeHelpers.cpp
runtime/vm/FFITypeHelpers.hpp
runtime/vm/OutOfLineINL_java_lang_invoke_NativeMethodHandle.cpp

I think we still want to delete those files with this change.

Also, please capitalize the first word of your commit message summaries; see the 'Commit Guidelines' section of https://github.com/eclipse/omr/blob/master/CONTRIBUTING.md.

Signed-off-by: Subhrodip Mohanta <subhrodipmohanta@gmail.com>
Signed-off-by: Subhrodip Mohanta <subhrodipmohanta@gmail.com>
Signed-off-by: Subhrodip Mohanta <subhrodipmohanta@gmail.com>
@ohbus
Copy link
Author

ohbus commented Jul 15, 2021

@keithc-ca Sorry for the delayed response, have been caught up real bad.

Please let me know of any changes that are required.

Thanks.

Signed-off-by: Subhrodip Mohanta <subhrodipmohanta@gmail.com>
@ohbus
Copy link
Author

ohbus commented Aug 17, 2021

Hey @keithc-ca It has been few weeks since this PR has been inactive.

Please do let me know after you review it if you would like some changes to it.

@keithc-ca
Copy link
Contributor

From my last review, I only have one requested change: see #12603 (comment).

@keithc-ca
Copy link
Contributor

@ohbus, do you anticipate having time to finish this soon?

@ohbus
Copy link
Author

ohbus commented Sep 16, 2021

@ohbus, do you anticipate having time to finish this soon?

So sorry @keithc-ca, somehow I have missed this notification. Let me finish this very soon.

Signed-off-by: Subhrodip Mohanta <subhrodipmohanta@gmail.com>
Copy link
Author

@ohbus ohbus left a comment

Choose a reason for hiding this comment

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

There were no conflicts when I requested for a review.

Let me know when you will be ready.

Thanks.

@keithc-ca
Copy link
Contributor

Other than the comment I made earlier, I think all that is required is to address the merge conflicts.

FFITypeHelpers.cpp and FFITypeHelpers.hpp must not be deleted, but the block conditional on J9VM_OPT_PANAMA in FFITypeHelpers.hpp can be removed.

@keithc-ca
Copy link
Contributor

@ohbus Are you planning to complete this?

@ohbus
Copy link
Author

ohbus commented Nov 29, 2021

Oh yes!

@keithc-ca
Copy link
Contributor

@ohbus Any progress on this? The longer we delay, the larger the list of conflicts is likely to grow.

@keithc-ca
Copy link
Contributor

No response from originator since #12603 (comment) - closing.

@keithc-ca keithc-ca closed this Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove PANAMA code
5 participants