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

WIP: Rename codeGen to cg #5661

Closed
wants to merge 1 commit into from
Closed

WIP: Rename codeGen to cg #5661

wants to merge 1 commit into from

Conversation

pkrc267
Copy link

@pkrc267 pkrc267 commented Nov 7, 2020

This patch makes all instances of CodeGenerator variables from codeGen to cg.

Issue: #5594
Signed-off-by: Prakhar Yadav [email protected]

@pkrc267
Copy link
Author

pkrc267 commented Nov 8, 2020

@0xdaryl 2 tests are failing for x86-64 macOS. Can you please guide me in fixing them! These are the ones:

31: [  FAILED  ] 2 tests, listed below:
31: [  FAILED  ] PortSockTest.poll_functionality_basic
31: [  FAILED  ] PortSockTest.poll_functionality_many_sockets

@fjeremic
Copy link
Contributor

fjeremic commented Nov 8, 2020

@0xdaryl 2 tests are failing for x86-64 macOS.

I believe those are due to #5581. They shouldn't be related to this change.

@pkrc267
Copy link
Author

pkrc267 commented Nov 9, 2020

Okay, thanks for clarifying. So how to proceed from here?

Copy link
Contributor

@0xdaryl 0xdaryl left a comment

Choose a reason for hiding this comment

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

This change generally looks good. Thanks for contributing.

You should sweep through this PR and find all patterns like this cg = cg() as they will fail to compile (depending on the context you should probably replace this with cg = this->cg(). I highlighted a few places where this needs to be changed in this PR, but I might have missed some so please review.

I won't launch a genie build until those places are fixed because I know it will fail.

This PR should also be squashed into a single commit. Perhaps once you fix the cg = this->cg() issue you can squash before pushing. I will then launch a genie build on that.

compiler/riscv/codegen/RVSystemLinkage.cpp Outdated Show resolved Hide resolved
compiler/riscv/codegen/RVSystemLinkage.cpp Outdated Show resolved Hide resolved
compiler/aarch64/codegen/ARM64SystemLinkage.cpp Outdated Show resolved Hide resolved
compiler/arm/codegen/ARMSystemLinkage.cpp Outdated Show resolved Hide resolved
compiler/arm/codegen/ARMSystemLinkage.cpp Outdated Show resolved Hide resolved
@0xdaryl 0xdaryl self-assigned this Nov 9, 2020
This patch makes all instances of CodeGenerator variables
from `codeGen` to `cg`, and resolves issues of object allocation
by replacing
`CodeGenerator *cg = cg()`
to
`CodeGenerator *cg = this->cg()`

Issue: eclipse#5594
Signed-off-by: Prakhar Yadav <[email protected]>
@0xdaryl
Copy link
Contributor

0xdaryl commented Nov 9, 2020

@genie-omr build all

@0xdaryl
Copy link
Contributor

0xdaryl commented Nov 9, 2020

Please address compilation failures in the CI builds. Looks like a couple of cg/cg() instances were missed on the ARM/AArch64 platforms.

@0xdaryl
Copy link
Contributor

0xdaryl commented Feb 1, 2021

Are you still planning on making progress with this PR? If not in the near term, may I suggest you close this PR while you work on addressing the issues in your private fork, and then open a PR again when the changes are ready for review. Thanks.

@0xdaryl
Copy link
Contributor

0xdaryl commented Feb 4, 2021

I am closing this as stale as the last two inquiries have gone unanswered. If you make further progress in your private fork and wish to contribute it please feel free to open a PR again in the future. Thanks for the contribution.

@0xdaryl 0xdaryl closed this Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants