flang-compiler / f18-llvm-project

Fork of llvm/llvm-project for f18. In sync with f18-mlir and f18.
http://llvm.org
28 stars 16 forks source link

Name mangling issue in mixed C and Fortran code base #1316

Open pmccormick opened 2 years ago

pmccormick commented 2 years ago

Testing the latest fir-dev and the new Flang driver using the sequential version of the Cloverleaf proxy app that uses a combination of C and Fortran 90 (note the use of submodules -- you want to use the CloverLeaf_Serial submodule and subdirectory).

First off, it looks like the new driver is working perfectly, but it appears that there is an issue w/ Flang name mangling C functions in way that causes a link time failure due to what appears to be a naming conflict between Clang produced objects and Flang's internal function name mangling.

Using this build command from the Cloverleaf_Serial subdirectory:

$ make COMPILER=GNU MPI_COMPILER=flang-new C_MPI_COMPILER=clang

In pack_kernel_c.o, nm shows that Clang produces symbols (clover_pack_message_leftc):

$ nm pack_kernel_c.o| grep pack_message_left
0000000000000000 T clover_pack_message_left_c_
0000000000000460 T clover_unpack_message_left_c_

While Flang is generating a non-matching mangled name:

/usr/bin/ld: /tmp/clover-4e0b74.o: in function `_QMclover_modulePclover_pack_left':
/home/pat/Projects/kitsune/proxy-tests/cloverleaf/CloverLeaf_Serial/./clover.f90:689: undefined reference to `_QPclover_pack_message_left_c'
/usr/bin/ld: /home/pat/Projects/kitsune/proxy-tests/cloverleaf/CloverLeaf_Serial/./clover.f90:701: undefined reference to `_QPclover_pack_message_left_c'
/usr/bin/ld: /home/pat/Projects/kitsune/proxy-tests/cloverleaf/CloverLeaf_Serial/./clover.f90:713: undefined reference to `_QPclover_pack_message_left_c'
/usr/bin/ld: /home/pat/Projects/kitsune/proxy-tests/cloverleaf/CloverLeaf_Serial/./clover.f90:725: undefined reference to `_QPclover_pack_message_left_c'
/usr/bin/ld: /home/pat/Projects/kitsune/proxy-tests/cloverleaf/CloverLeaf_Serial/./clover.f90:737: undefined reference to `_QPclover_pack_message_left_c'
/usr/bin/ld: /tmp/clover-4e0b74.o:/home/pat/Projects/kitsune/proxy-tests/cloverleaf/CloverLeaf_Serial/./clover.f90:749: more undefined references to `_QPclover_pack_message_left_c' follow
/usr/bin/ld: /tmp/clover-4e0b74.o: in function `_QMclover_modulePclover_unpack_left':
/home/pat/Projects/kitsune/proxy-tests/cloverleaf/CloverLeaf_Serial/./clover.f90:1087: undefined reference to `_QPclover_unpack_message_left_c'
/usr/bin/ld: /home/pat/Projects/kitsune/proxy-tests/cloverleaf/CloverLeaf_Serial/./clover.f90:1099: undefined reference to `_QPclover_unpack_message_left_c'
/usr/bin/ld: /home/pat/Projects/kitsune/proxy-tests/cloverleaf/CloverLeaf_Serial/./clover.f90:1111: undefined reference to `_QPclover_unpack_message_left_c'
/usr/bin/ld: /home/pat/Projects/kitsune/proxy-tests/cloverleaf/CloverLeaf_Serial/./clover.f90:1123: undefined reference to `_QPclover_unpack_message_left_c'
/usr/bin/ld: /home/pat/Projects/kitsune/proxy-tests/cloverleaf/CloverLeaf_Serial/./clover.f90:1135: undefined reference to `_QPclover_unpack_message_left_c'
/usr/bin/ld: /tmp/clover-4e0b74.o:/home/pat/Projects/kitsune/proxy-tests/cloverleaf/CloverLeaf_Serial/./clover.f90:1147: more undefined references to `_QPclover_unpack_message_left_c' follow
...
svedan commented 2 years ago

@clementval can you review this? I thought this was addressed in fir-dev. Related issue #1317.

schweitzpgi commented 2 years ago

Which driver is this using? flang-new?

The driver needs to add the external name conversion pass to the pipeline. Without that pass, the internal unique names will be emitted.

clementval commented 2 years ago

As @schweitzpgi mentioned the external mangling pass is not in the default pipepline.

pmccormick commented 2 years ago

Yes, this is with the new driver.

banach-space commented 2 years ago

@schweitzpgi Do you mean ExternalNameConversionPass ? And should this change be specific to any particular driver, e.g. flang-new?

I think that making pass-pipeline definition independent of the driver might make code re-use easier. Just my 2p.

clementval commented 2 years ago

It was intentional to not have it by default in bbc/tco since those are testing tools. I know that Steve's driver includes it. I guess flang-new needs it as well.

banach-space commented 2 years ago

I think that it is important that bbc/tco and flang-new remain compatible. We should always be able to assume that bbc file.f90 -o - | tco is equivalent to flang-new -emit-llvm file.f90.

schweitzpgi commented 2 years ago

I think that it is important that bbc/tco and flang-new remain compatible. We should always be able to assume that bbc file.f90 -o - | tco is equivalent to flang-new -emit-llvm file.f90.

Not really. This is a case where "flang" requires a different behavior than the test tools because "flang" must interoperate with other tools. The test tools do not require this interoperability, do not need the added complexity, and should not artificially be made to do so.