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 the use of the C Preprocessor on NASM assembly files #3243

Merged
merged 4 commits into from
Oct 12, 2018

Conversation

nbhuiyan
Copy link
Member

@nbhuiyan nbhuiyan commented Oct 11, 2018

NASM assembly files will no longer need the C Preprocessor. Going forward, there will be no intermediate source files to look at when debugging any of the JIT assembly code. Furthermore, OMR will not need to update its CMake files to handle the temporary pnasm extention (see eclipse-omr/omr#3042)

Depends on: #3237

WIP because of dependency on the PR above which enables writing constants in jilconsts.inc with %define directives.

@0xdaryl
Copy link
Contributor

0xdaryl commented Oct 12, 2018

J9VM_OPT_TEMP_NEW_INTERFACE_INVOCATION is not defined in any build spec (confirmed by @pshipton in the OpenJ9 Slack discussion here). Can you simply remove it and fold code assuming it's not defined?

Other changes look good.

J9VM_OPT_TEMP_NEW_INTERFACE_INVOCATION is not defined in any build
spec, and therefore can be removed from X86PicBuilder with the
assumption that it is never defined.

Signed-off-by: Nazim Uddin Bhuiyan <[email protected]>
NASM's preprocessor is capable of handling the ifdef check the
C preprocessor was doing. The macro NASM preproceesor will use,
ASM_J9VM_INTERP_COMPRESSED_OBJECT_HEADER is defined in
jilconsts.inc.

Signed-off-by: Nazim Uddin Bhuiyan <[email protected]>
X86PicBuilder.pnasm and X86Unresolveds.pnasm no longer need to go
through the C preprocessor. Also updated the filenames in the
makefiles.

Signed-off-by: Nazim Uddin Bhuiyan <[email protected]>
NASM files no longer need to to go through the C Preprocessor.

Signed-off-by: Nazim Uddin Bhuiyan <[email protected]>
@nbhuiyan nbhuiyan changed the title WIP: Remove the use of the C Preprocessor on NASM assembly files Remove the use of the C Preprocessor on NASM assembly files Oct 12, 2018
@nbhuiyan
Copy link
Member Author

@0xdaryl addressed your concerns regarding the never-defined macro, rebased, and did some local testing. Ready for review.

@0xdaryl
Copy link
Contributor

0xdaryl commented Oct 12, 2018

Jenkins test sanity

@0xdaryl 0xdaryl self-assigned this Oct 12, 2018
@0xdaryl
Copy link
Contributor

0xdaryl commented Oct 12, 2018

macOS failure is infrastructure, and the Win32 tests all passed but failed notification (infra issue). I don't see any problems related to this PR. Merging...

@0xdaryl 0xdaryl merged commit 2f039d8 into eclipse-openj9:master Oct 12, 2018
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.

2 participants