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

Override needRelocationsForLookupEvaluationData #7793

Merged
merged 1 commit into from Nov 27, 2019

Conversation

dchopra001
Copy link
Contributor

This commit overrides the needRelocationsForLookupEvaluationData query found in the OMR codegenerator. The query is used to decide if a reloction record is needed when generating code for the lookup IL in power's code generator (it is needed for AOT and Remote compiles). Since OMR is not aware of remote compilations, overriding the query here will allow Openj9 to make a decision on whether a relocation record is needed.

Signed-off-by: Dhruv Chopra Dhruv.C.Chopra@ibm.com

This commit overrides the needRelocationsForLookupEvaluationData
query found in the OMR codegenerator. The query is used to decide if
a reloction record is needed when generating code for the lookup IL in
power's code generator (it is needed for AOT and Remote compiles).
Since OMR is not aware of remote compilations, overriding the query here
will allow Openj9 to make a decision on whether a relocation record is
needed.

Signed-off-by: Dhruv Chopra <Dhruv.C.Chopra@ibm.com>
@dchopra001
Copy link
Contributor Author

See eclipse/omr#4441 for the PR that introduces this query in OMR.

@dchopra001
Copy link
Contributor Author

@ymanton Can I get a review please?

Copy link
Member

@ymanton ymanton left a comment

Choose a reason for hiding this comment

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

LGTM. We can deal with this once the OMR change lands.

@mpirvu mpirvu mentioned this pull request Nov 22, 2019
19 tasks
Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

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

LGTM

@mpirvu
Copy link
Contributor

mpirvu commented Nov 26, 2019

Jenkins test sanity all jdk8,jdk11

@dchopra001
Copy link
Contributor Author

dchopra001 commented Nov 26, 2019

travis build failed with:

E: Unable to locate package g++-7
178E: Couldn't find any package by regex 'g++-7'
179E: Unable to locate package gcc-7
apt-get.diagnostics
180apt-get install failed
372The command "sudo -E apt-get -yq --no-install-suggests --no-install-recommends $(travis_apt_get_options) install autoconf ca-certificates ccache cpio file g++-7 gcc-7 git git-core libasound2-dev libcups2-dev libdwarf-dev libelf-dev libfreetype6-dev libnuma-dev libx11-dev libxext-dev libxrender-dev libxt-dev libxtst-dev libxrandr-dev make nasm pkg-config realpath ssh unzip wget zip clang-3.8 llvm-3.8 libclang-3.8-dev llvm-3.8-dev" failed and exited with 100 during .

Looks like an infrastructure issue.

@dchopra001
Copy link
Contributor Author

The Mac build also failed during cloning:

ERROR: Error cloning remote repo 'origin'

@mpirvu Can we try launching these tests again?

@mpirvu
Copy link
Contributor

mpirvu commented Nov 26, 2019

Jenkins test sanity osx jdk8

@dchopra001
Copy link
Contributor Author

All builds except for travis passed. The travis failure is the one in: #7793 (comment)

@ymanton @mpirvu Is it possible to try the travis build again? Or can we merge without it, considering all the other builds that have passed?

@mpirvu
Copy link
Contributor

mpirvu commented Nov 27, 2019

Travis failed due to infra. Sanity tests passed, hence merging.

@mpirvu mpirvu merged commit 56c071b into eclipse-openj9:master Nov 27, 2019
@dchopra001 dchopra001 deleted the lookupEvalQuery branch July 13, 2021 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants