-
Notifications
You must be signed in to change notification settings - Fork 138
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
patch flang runtime library #980
patch flang runtime library #980
Conversation
runtime/flang/curdir.c
Outdated
| @@ -102,5 +104,8 @@ void __fort_gethostname(host) char *host; | |||
| } | |||
| p = un.nodename; | |||
| } | |||
| #else | |||
| strcpy(p, "localhost"); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be an equivalent for gethostname in the Win32 or Winsock API, right?
Can you keep the support for -curhost on Windows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is, but I haven't put it in yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that should be good for now; I will add support for this later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will add TODO in a separate patch.
|
@kaadam Are you going to look at this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, nit: Could you add a description about what you have done this patch? Thanks.
|
Could you squash those 18 commits into one? |
|
@pawosm-arm do I need feedback from shivaramaarao first? Or do you have write access? |
|
@shivaramaarao has to approve for the x86_64/amd64 platform. @pawosm-arm, @bryanpkc or myself can approve for aarch64. |
|
I'll wait until he approves, then squash them. |
* modify function so that they work on windows. * add OMP alias to forward fortran OMP calls to c library
d9ac7ec to
06feecf
Compare
|
@bryanpkc @shivaramaarao @kiranchandramohan Could you please take a look at this PR? Thanks in advance! |
|
@kiranchandramohan is there any chance that this will be merged soon? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few quick comments.
|
I will run a few tests tomorrow and if OK will accept. Does @bryanpkc have further comments? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to merge this PR before this Wednesday's general Flang call if there are no further comments.
|
As per the new pull request policy, we only need 2 reviewers (1 windows + 1 other) to review and approve windows PRs. This patch meets the criteria and hence can be merged if the CI is passing. Thanks @xoviat for working on this patch. https://github.com/flang-compiler/flang/wiki/Community#the-pull-request-process |
|
Sorry I missed the messages on this PR. Thanks for merging @kiranchandramohan. |
|
@xoviat For future reference please following the coding convention and use 2-space indents. |
patch the runtime library so that it works on windows.