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

Resolves building issues with Intel compilers (ifx/icx) #1823

Merged
merged 4 commits into from
Dec 8, 2023
Merged

Conversation

mgs-52
Copy link
Contributor

@mgs-52 mgs-52 commented Mar 7, 2023

Adding/Replacing missing prototypes. Remove unsupported/bogus compiler switches.

TYPE: Bug fix

KEYWORDS: Missing Prototypes

SOURCE: Alexander Knyazev

DESCRIPTION OF CHANGES:
Problem: Most C code written long ago. No prototypes were used.

Solution:
Add missing prototypes.

LIST OF MODIFIED FILES:

external/io_grib1/grib1_util/read_grib.c
external/io_grib1/grib1_util/write_grib.c
external/io_grib1/grib1_util/read_grib.h
external/io_grib1/MEL_grib1/FTP_getfile.c
external/io_grib1/MEL_grib1/gribfuncs.h
external/io_grib1/MEL_grib1/gribgetbds.c
external/io_grib1/MEL_grib1/upd_child_errmsg.c
external/io_grib1/MEL_grib1/gribhdr2file.c
external/io_grib1/MEL_grib1/init_enc_struct.c
external/io_grib1/MEL_grib1/init_gribhdr.c
external/io_grib1/MEL_grib1/init_struct.c
external/io_grib1/MEL_grib1/ld_enc_input.c
external/io_grib1/MEL_grib1/ld_enc_lookup.c
external/io_grib1/MEL_grib1/map_lvl.c
external/io_grib1/MEL_grib1/map_parm.c
external/io_grib1/MEL_grib1/prt_inp_struct.c
external/io_grib1/MEL_grib1/prt_badmsg.c
external/io_grib1/MEL_grib1/set_bytes.c
external/io_grib1/MEL_grib1/ld_grib_origctrs.c
external/io_grib1/gribmap.c
external/io_grib1/grib1_routines.c
external/io_grib1/gribmap.h
external/io_grib1/trim.c
external/io_grib1/trim.h
external/io_grib_share/get_region_center.c
external/io_grib_share/open_file.c
external/RSL_LITE/gen_comms.c
external/RSL_LITE/c_code.c
external/RSL_LITE/buf_for_proc.c
external/RSL_LITE/rsl_malloc.c
external/RSL_LITE/rsl_bcast.c
external/RSL_LITE/task_for_point.c
external/RSL_LITE/period.c
external/RSL_LITE/swap.c
external/RSL_LITE/cycle.c
external/RSL_LITE/rsl_lite.h
tools/protos.h
tools/reg_parse.c
tools/misc.c
tools/symtab_gen.c
tools/gen_config.c
tools/sym.c
tools/gen_streams.c
tools/gen_irr_diag.c
tools/sym.h
frame/pack_utils.c
arch/configure.defaults

TESTS CONDUCTED:

  1. Compilation with Intel oneAPI compiler works with these changes.
  2. The Jenkins tests are all passing.

RELEASE NOTE: This PR fixed some c code by adding and replacing missing prototypes to allow Intel oneAPI compiler to work. Remove unsupported/bogus compiler switches.

Adding/Replacing missing prototypes. Remove unsupported/bogus compiler switches.
@mgs-52 mgs-52 requested a review from a team as a code owner March 7, 2023 21:04
@weiwangncar
Copy link
Collaborator

Jenkins test results:

Test Type              | Expected  | Received |  Failed
= = = = = = = = = = = = = = = = = = = = = = = =  = = = =
Number of Tests        : 23           24
Number of Builds       : 60           57
Number of Simulations  : 158           150        0
Number of Comparisons  : 95           86        0

Failed Simulations are: 
None
Which comparisons are not bit-for-bit: 
None

@mgs-52 mgs-52 changed the title Resolves building issues with Intel LLVM compiler (ifx/icx) Resolves building issues with Intel compilers (ifx/icx) Mar 16, 2023
@mgduda mgduda self-requested a review April 13, 2023 16:10
@islas
Copy link
Collaborator

islas commented Aug 15, 2023

@mgs-52 This should target develop

frame/pack_utils.c Outdated Show resolved Hide resolved
tools/symtab_gen.c Outdated Show resolved Hide resolved
@mgs-52 mgs-52 requested a review from islas August 16, 2023 09:00
@weiwangncar weiwangncar changed the base branch from master to develop August 23, 2023 01:21
islas
islas previously approved these changes Aug 24, 2023
@weiwangncar
Copy link
Collaborator

@islas @mgduda I just compiled this code on Derecho. With these modifications, one can compile WRF using the default modules. Would it be ok if we move this PR for release-4.5.2 release?

@islas
Copy link
Collaborator

islas commented Oct 2, 2023

Yes, this should go into the next release. My own testing with the latest Intel compilers from their download page also works with these updates

@weiwangncar weiwangncar changed the base branch from develop to release-v4.5.2 October 2, 2023 19:24
@weiwangncar weiwangncar dismissed islas’s stale review October 2, 2023 19:24

The base branch was changed.

@weiwangncar
Copy link
Collaborator

@mgs-52 Can you let us know your affiliation, so that we can fill the SOURCE section of the PR? Thanks.

@islas
Copy link
Collaborator

islas commented Oct 17, 2023

@mgs-52 Any updates? I think affiliation/SOURCE is the last change needed

@mgs-52
Copy link
Contributor Author

mgs-52 commented Oct 18, 2023

Well. Technically I'm working in Intel. However this change I did on my own so fill free to Include Intel or not :).

@weiwangncar
Copy link
Collaborator

Well. Technically I'm working in Intel. However this change I did on my own so fill free to Include Intel or not :).

How about your name? Would you want to include your real name? With that, we don't have to add Intel.

@mgs-52
Copy link
Contributor Author

mgs-52 commented Oct 18, 2023

Alexander (Alex) Knyazev.

@islas
Copy link
Collaborator

islas commented Oct 27, 2023

@mgduda When you get a chance, could you take a look at this? Ignoring whitespace changes most of the edits are rearrangement of function order and fixing bad function signatures (implicit returns and Fortran-esque argument typing)

@ahheo
Copy link
Contributor

ahheo commented Nov 26, 2023

@mgs-52 May you please do the same job for the codes under WRF/chem (especially those of KPP) as well? I guess WRF-chem users may also get trouble with ifx/icx. Actually, I have done the code modification dealing with the issues of missing of prototypes and function order, and successfully built the WRF with WRF_CHEM=1 and WRF_KPP=1 using the newest Intel compilers. Well, as this is of the same issue that this pull is dealing with, I therefore suggest to complete the same job taking into account WRF-chem before the pull to be merged. What do you think @weiwangncar @islas?

@weiwangncar
Copy link
Collaborator

@ahheo Can you point out the files you modified for ifx/icx in chem/ directory? I think it is also ok to do another PR to address WRF-Chem.

@ahheo
Copy link
Contributor

ahheo commented Nov 28, 2023

@weiwangncar I have done another PR #1942 as you suggested.

@weiwangncar weiwangncar merged commit ae0906e into wrf-model:release-v4.5.2 Dec 8, 2023
@smileMchen
Copy link
Collaborator

@mgs-52 Alexander Knyazev,
Would you please tell where you are working? We would like to have this information for release note. Thanks.

@mgs-52
Copy link
Contributor Author

mgs-52 commented Dec 18, 2023

@mgs-52 Alexander Knyazev, Would you please tell where you are working? We would like to have this information for release note. Thanks.

Intel, but I as stated above this doesn't need to be included since I did it on my own.

Fergui added a commit to openwfm/WRF-SFIRE that referenced this pull request Jan 5, 2024
Fixes compilation using Intel oneAPI compiler
@weiwangncar
Copy link
Collaborator

weiwangncar commented Jan 6, 2024

@mgs-52 Will these icx compiler flags, '-Wno-implicit-function-declaration -Wno-incompatible-function-pointer-types', help with any of the old c code changed in this PR? See PR-1972 for WRFDA compilation change. Just curious.

@mgs-52
Copy link
Contributor Author

mgs-52 commented Jan 7, 2024

@mgs-52 Will these icx compiler flags, '-Wno-implicit-function-declaration -Wno-incompatible-function-pointer-types', help with any of the old c code changed in this PR? See PR-1972 for WRFDA compilation change. Just curious.
They do, but I'd say it's a clutch: better make the code "compatible" with the ("C") standard rather than instruct compiler to ignore "incompatibilities".

vlakshmanan-scala pushed a commit to scala-computing/WRF that referenced this pull request Apr 4, 2024
Adding/Replacing missing prototypes. Remove unsupported/bogus compiler switches.

TYPE: Bug fix

KEYWORDS: Missing prototypes, old unsupported compile options

SOURCE:  Alexander Knyazev

DESCRIPTION OF CHANGES:
Problem:  Most C code written long ago.  No prototypes were used.

Solution:
Add missing prototypes. 

LIST OF MODIFIED FILES: 

external/io_grib1/grib1_util/read_grib.c
external/io_grib1/grib1_util/write_grib.c
external/io_grib1/grib1_util/read_grib.h
external/io_grib1/MEL_grib1/FTP_getfile.c
external/io_grib1/MEL_grib1/gribfuncs.h
external/io_grib1/MEL_grib1/gribgetbds.c
external/io_grib1/MEL_grib1/upd_child_errmsg.c
external/io_grib1/MEL_grib1/gribhdr2file.c
external/io_grib1/MEL_grib1/init_enc_struct.c
external/io_grib1/MEL_grib1/init_gribhdr.c
external/io_grib1/MEL_grib1/init_struct.c
external/io_grib1/MEL_grib1/ld_enc_input.c
external/io_grib1/MEL_grib1/ld_enc_lookup.c
external/io_grib1/MEL_grib1/map_lvl.c
external/io_grib1/MEL_grib1/map_parm.c
external/io_grib1/MEL_grib1/prt_inp_struct.c
external/io_grib1/MEL_grib1/prt_badmsg.c
external/io_grib1/MEL_grib1/set_bytes.c
external/io_grib1/MEL_grib1/ld_grib_origctrs.c
external/io_grib1/gribmap.c
external/io_grib1/grib1_routines.c
external/io_grib1/gribmap.h
external/io_grib1/trim.c
external/io_grib1/trim.h
external/io_grib_share/get_region_center.c
external/io_grib_share/open_file.c
external/RSL_LITE/gen_comms.c
external/RSL_LITE/c_code.c
external/RSL_LITE/buf_for_proc.c
external/RSL_LITE/rsl_malloc.c
external/RSL_LITE/rsl_bcast.c
external/RSL_LITE/task_for_point.c
external/RSL_LITE/period.c
external/RSL_LITE/swap.c
external/RSL_LITE/cycle.c
external/RSL_LITE/rsl_lite.h
tools/protos.h
tools/reg_parse.c
tools/misc.c
tools/symtab_gen.c
tools/gen_config.c
tools/sym.c
tools/gen_streams.c
tools/gen_irr_diag.c
tools/sym.h
frame/pack_utils.c
arch/configure.defaults 

TESTS CONDUCTED:
1. Compilation with Intel oneAPI compiler works with these changes.
2. The Jenkins tests are all passing.

RELEASE NOTE: This PR fixed some c code by adding and replacing missing prototypes. This also allows the new Intel oneAPI compiler to work. Remove unsupported/bogus compiler switches.
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.

5 participants