banach-space wrote:
@kkwli and @DanielCChen - could you review
https://github.com/llvm/llvm-project/pull/75816 and see whether that makes
sense to you? I am referring specifically to the documentation that @mjklemm is
kindly adding. Hopefully that will help folks avoid similar issues in the
kkwli wrote:
Yep the situation is something like using `flang-new` to link the C and Fortran
objects.
```
$ cat main.c
void fsub();
int main() { fsub(); }
$ cat sub.f90
subroutine fsub()
end subroutine
$ flang-new -c sub.f90 -fno-underscoring
$ clang -c main.c
$ flang-new main.o sub.o
DanielCChen wrote:
The test cases actually have C main indeed and call to Fortran procedures as
opposed to what I thought (the other way around). Adding `-fno-fortran-main`
fixed all of them!
May be I missed it when reading through the comments of this PR, why there is a
definition of
banach-space wrote:
> I see. So Fortran and C interoperability of F2003/F2008 is not supported yet
> in Flang?
That's not really what I had in mind. It worked for you so everything that's
needed is there, but no upstream testing indicates that you might be the first
person trying it. And
DanielCChen wrote:
I see. So Fortran and C interoperability of F2003/F2008 is not supported yet in
Flang? Those ~100ish regression test cases we have were passing before this PR
though.
Unfortunately, those test cases are not made public available yet. I think I
can copy the source code of
DanielCChen wrote:
@mjklemm This PR caused some regressions of C-interop test cases in our local
test run. The test cases typically have a Fortran main (compiled with Flang)
that calls a C function (compiled with clang). The linking is by `flang-new`.
The error looks like:
```
ld.lld: error:
mjklemm wrote:
> We are also seeing the same issue when linking on Mac regarding the ld:
> unknown options: --whole-archive
Is there an equivalent option for the MacOS linker?
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing
banach-space wrote:
> It seems like flang-new when being used as a linker with -shared included
> Fortran_main in the shared library. This seems wrong to me.
I am trying to recall the rationale behind that, but it's been a while :(
Here's a relevant discussion/bug that hasn't been resolved
mjklemm wrote:
Thanks for the reproducer.
> The error shows up when linking a C program with a Fortran shared library, so
> maybe you weren't enabling building shared libraries?
I was building OpenBLAS using the "old" Makefile-based build. There the issue
indeed does not happen. When I
rj-jesus wrote:
Chipping into the discussion, since this patch I can also no longer build
OpenBLAS or PETSc. OpenBLAS for example fails with
```
$ clang -v -O3 -mcpu=native -DHAVE_C11 -Wall -DF_INTERFACE_GFORT -fPIC
-DSMP_SERVER -DNO_WARMUP -DMAX_CPU_NUMBER=72 -DMAX_PARALLEL_NUMBER=1
mjklemm wrote:
Ok, got it! I'm fione with reverting this patch and seeking a better solution.
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
tblah wrote:
Simple reproducer:
main.c
```
int main(void) {
// call fortran code
return 0;
}
```
fortran.f90
```
function pow(a, b)
real :: a, b, pow
pow = a ** b
end function
```
```
clang -c main.c -o main.o
flang-now -c fortran.f90 -o fortran.o
flang-new fortran.o main.o -o out
```
tblah wrote:
Yes they are mixed source projects. They worked previously because the main
function from `Frotran_main` was silently ignored. I don't know if this was
accidental or not, but it tended to do the right thing for mixed source
projects because if the user intended to use
banach-space wrote:
> Since this patch, I can no longer build spec2006 gromacs and calculix because
> they supply their own main function in a C file, then link using flang-new:
> leading to another definition of `main()` from the runtime. Do I need to use
> a special flag to avoid linking to
tblah wrote:
Since this patch, I can no longer build spec2006 gromacs and calculix because
they supply their own main function in a C file, then link using flang-new:
leading to another definition of `main()` from the runtime. Do I need to use a
special flag to avoid linking to
psteinfeld wrote:
> @mjklemm, after this change was integrated, the test Driver/ctofortran no
> longer succeeds. This test gets run when you call `make check-flang`. I'm not
> sure why the pre- and post-build checks did not run check-flang.
I've submitted #73738 to fix this.
psteinfeld wrote:
@mjklemm, after this change was integrated, the test Driver/ctofortran no
longer succeeds. This test gets run when you call `make check-flang`. I'm not
sure why the pre- and post-build checks did not run check-flang.
https://github.com/llvm/llvm-project/pull/73124
mjklemm wrote:
Thanks all for your reviews and the all the feedback you have provided! Much
appreciated!
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/kparzysz closed
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/mjklemm updated
https://github.com/llvm/llvm-project/pull/73124
>From 2a2693364cb8e9b657b9ff54aa78df0466b55fe4 Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 14:22:20 +0100
Subject: [PATCH 01/16] Let the linker fail on multiple definitions of main()
---
@@ -977,14 +977,61 @@ bool tools::addOpenMPRuntime(ArgStringList ,
const ToolChain ,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain ,
+void tools::addFortranRuntimeLibs(const ToolChain , const ArgList ,
@@ -977,14 +977,61 @@ bool tools::addOpenMPRuntime(ArgStringList ,
const ToolChain ,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain ,
+void tools::addFortranRuntimeLibs(const ToolChain , const ArgList ,
@@ -977,14 +977,61 @@ bool tools::addOpenMPRuntime(ArgStringList ,
const ToolChain ,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain ,
+void tools::addFortranRuntimeLibs(const ToolChain , const ArgList ,
@@ -977,14 +977,61 @@ bool tools::addOpenMPRuntime(ArgStringList ,
const ToolChain ,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain ,
+void tools::addFortranRuntimeLibs(const ToolChain , const ArgList ,
https://github.com/mjklemm updated
https://github.com/llvm/llvm-project/pull/73124
>From 2a2693364cb8e9b657b9ff54aa78df0466b55fe4 Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 14:22:20 +0100
Subject: [PATCH 01/14] Let the linker fail on multiple definitions of main()
---
@@ -977,14 +977,63 @@ bool tools::addOpenMPRuntime(ArgStringList ,
const ToolChain ,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain ,
+void tools::addFortranRuntimeLibs(const ToolChain , const ArgList ,
mjklemm wrote:
Folks, I have made another attempt to improve this patch. @kparzysz with your
feedback in mind, I have now added a check if `--whole-archive` is active for
some reason. If so, flang will not add it to the link line again.
https://github.com/llvm/llvm-project/pull/73124
https://github.com/mjklemm updated
https://github.com/llvm/llvm-project/pull/73124
>From 2a2693364cb8e9b657b9ff54aa78df0466b55fe4 Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 14:22:20 +0100
Subject: [PATCH 01/13] Let the linker fail on multiple definitions of main()
---
https://github.com/kparzysz approved this pull request.
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -977,14 +977,51 @@ bool tools::addOpenMPRuntime(ArgStringList ,
const ToolChain ,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain ,
+void tools::addFortranRuntimeLibs(const ToolChain , const ArgList ,
@@ -977,14 +977,51 @@ bool tools::addOpenMPRuntime(ArgStringList ,
const ToolChain ,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain ,
+void tools::addFortranRuntimeLibs(const ToolChain , const ArgList ,
@@ -977,14 +977,51 @@ bool tools::addOpenMPRuntime(ArgStringList ,
const ToolChain ,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain ,
+void tools::addFortranRuntimeLibs(const ToolChain , const ArgList ,
@@ -977,14 +977,51 @@ bool tools::addOpenMPRuntime(ArgStringList ,
const ToolChain ,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain ,
+void tools::addFortranRuntimeLibs(const ToolChain , const ArgList ,
mjklemm wrote:
> I think for Windows the easy thing to do here is just to add
> `/WHOLEARCHIVE:...` here anyway, using the same logic as in
> processVSRuntimeLibrary(); E.g
>
> ```
> void tools::addFortranRuntimeLibs(const ToolChain , const ArgList ,
> //need to add args here so we can
DavidTruby wrote:
I think for Windows the easy thing to do here is just to add
`/WHOLEARCHIVE:...` here anyway, using the same logic as in
processVSRuntimeLibrary(); E.g
```
void tools::addFortranRuntimeLibs(const ToolChain , const ArgList ,
//need to add args here so we can search it
mjklemm wrote:
FYI: Rebased and resolved conflict flagged by GitHub. Alas, this means that I
have lost the change to also make the linker fail on Windows. I've sent a
request to the authors of the code to perform linking on Windows to help me
figure out what to do.
https://github.com/mjklemm updated
https://github.com/llvm/llvm-project/pull/73124
>From 2a2693364cb8e9b657b9ff54aa78df0466b55fe4 Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 14:22:20 +0100
Subject: [PATCH 01/11] Let the linker fail on multiple definitions of main()
---
banach-space wrote:
> > LGTM, thank you for taking care of this
> > Dare I ask - what's "dupes"? I only found
> > [dupe](https://dictionary.cambridge.org/dictionary/english/dupe). Also,
> > please wait for @kiranchandramohan to approve before merging this :)
>
> I used "dupes" in the sense
@@ -0,0 +1,15 @@
+! UNSUPPORTED: system-windows
+
+! RUN: %flang -x ir -o %t.c-object -c %S/Inputs/main_dupes.ll
+! RUN: %flang -o %t -c %s
+! RUN: not %flang -o %t.exe %t %t.c-object 2>&1
banach-space wrote:
That was just a nit - I am happy if you keep things
https://github.com/banach-space edited
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -0,0 +1,15 @@
+! UNSUPPORTED: system-windows
+
+! RUN: %flang -x ir -o %t.c-object -c %S/Inputs/main_dupes.ll
+! RUN: %flang -o %t -c %s
+! RUN: not %flang -o %t.exe %t %t.c-object 2>&1
mjklemm wrote:
I'd actually prefer to have a separate test for this, as a
mjklemm wrote:
> LGTM, thank you for taking care of this
>
> Dare I ask - what's "dupes"? I only found
> [dupe](https://dictionary.cambridge.org/dictionary/english/dupe). Also,
> please wait for @kiranchandramohan to approve before merging this :)
I used "dupes" in the sense of being
https://github.com/banach-space edited
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -0,0 +1,15 @@
+! UNSUPPORTED: system-windows
+
+! RUN: %flang -x ir -o %t.c-object -c %S/Inputs/main_dupes.ll
+! RUN: %flang -o %t -c %s
+! RUN: not %flang -o %t.exe %t %t.c-object 2>&1
banach-space wrote:
[nit] You could add a "valid"/"working" compilation
@@ -122,6 +122,7 @@
# the build directory holding that tool.
tools = [
ToolSubst("%flang", command=FindTool("flang-new"), unresolved="fatal"),
+ToolSubst("%clang", command=FindTool("clang"), unresolved="fatal"),
mjklemm wrote:
I have removed this in
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff e5cc3da6a9077548f613eee3aacc5e7b017c81f3
3b0090997023b1b6392bc23d386ace7c7cb796ce --
mjklemm wrote:
Done.
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/mjklemm updated
https://github.com/llvm/llvm-project/pull/73124
>From ba38aec7ac04c63fd5167908fe7f91d6ac7bceed Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 14:22:20 +0100
Subject: [PATCH 1/6] Let the linker fail on multiple definitions of main()
---
mjklemm wrote:
> Would it be possible to test this?
I have added a simple test. It checks that the linker indeed fails, but does
not check the actual linker error message. PLease let me know if that's
desired. I could then provide the error messages for LD and LLD. For other
banach-space wrote:
As this is a "test input" rather than a "test file", could you move it to
"flang/test/Driver/Inputs"?
https://github.com/llvm/llvm-project/pull/73124
___
cfe-commits mailing list
@@ -122,6 +122,7 @@
# the build directory holding that tool.
tools = [
ToolSubst("%flang", command=FindTool("flang-new"), unresolved="fatal"),
+ToolSubst("%clang", command=FindTool("clang"), unresolved="fatal"),
mjklemm wrote:
@banach-space I had to
https://github.com/mjklemm updated
https://github.com/llvm/llvm-project/pull/73124
>From ba38aec7ac04c63fd5167908fe7f91d6ac7bceed Mon Sep 17 00:00:00 2001
From: Michael Klemm
Date: Wed, 22 Nov 2023 14:22:20 +0100
Subject: [PATCH 1/5] Let the linker fail on multiple definitions of main()
---
52 matches
Mail list logo
