https://github.com/banach-space approved this pull request.
Thanks for addressing my feedback, LGTM
https://github.com/llvm/llvm-project/pull/100152
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
@@ -492,6 +493,18 @@ void Flang::addOffloadOptions(Compilation , const
InputInfoList ,
if (Args.hasArg(options::OPT_nogpulib))
CmdArgs.push_back("-nogpulib");
}
+
+ // For all the host OpenMP offloading compile jobs we need to pass the
targets
+ // information
https://github.com/banach-space approved this pull request.
https://github.com/llvm/llvm-project/pull/96742
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
banach-space wrote:
> > > > Who could be the right person to ask?
> > >
> > >
> > > I don't know. Open-source LLVM Flang meetings can be good place to ask
> > > this question.
> >
> >
> > Did you ask? What feedback did you get?
>
> @banach-space I asked question on flang-slack, I mentioned
banach-space wrote:
Given the growing number of OpenMP and/or "offloading" flags, I agree with
@AnastasiaStulova that it would be good to clarify the overall goal/design.
That's not clear to me.
Is there are reference implementation that Flang is meant to follow? For
example Clang or
https://github.com/banach-space approved this pull request.
https://github.com/llvm/llvm-project/pull/100343
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -492,6 +493,18 @@ void Flang::addOffloadOptions(Compilation , const
InputInfoList ,
if (Args.hasArg(options::OPT_nogpulib))
CmdArgs.push_back("-nogpulib");
}
+
+ // For all the host OpenMP offloading compile jobs we need to pass the
targets
+ // information
https://github.com/banach-space approved this pull request.
https://github.com/llvm/llvm-project/pull/99058
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/banach-space closed
https://github.com/llvm/llvm-project/pull/98517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/banach-space approved this pull request.
https://github.com/llvm/llvm-project/pull/98517
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/banach-space approved this pull request.
The driver logic makes sense, thank you! LGTM
https://github.com/llvm/llvm-project/pull/98083
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
@@ -333,6 +333,9 @@ void Flang::AddAMDGPUTargetArgs(const ArgList ,
StringRef Val = A->getValue();
CmdArgs.push_back(Args.MakeArgString("-mcode-object-version=" + Val));
}
+
+ const ToolChain = getToolChain();
+ TC.addClangTargetOptions(Args, CmdArgs,
@@ -8024,7 +8024,7 @@ def source_date_epoch : Separate<["-"],
"source-date-epoch">,
// CUDA Options
//===--===//
-let Visibility = [CC1Option] in {
+let Visibility = [CC1Option, FC1Option] in {
banach-space wrote:
> Would it be possible for you to investigate that? It really shouldn't be
> required if we can't help it.
+1
https://github.com/llvm/llvm-project/pull/96742
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
@@ -0,0 +1,9 @@
+; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s
banach-space wrote:
I have X86 disabled - I just double checked. And yes, this test works for me
just fine without the guard. Alexis, what error do you get when it fails for
@@ -0,0 +1,9 @@
+; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s
banach-space wrote:
Hm, ninja `check-mlir` worked for me just fine on Aarch64 樂 I think that
`mlir-translate` doesn't really care :) Put differently, `REQUIRES` is not
banach-space wrote:
> Clang for AMDGPU supports OpenMP and
> [HIP](https://clang.llvm.org/docs/HIPSupport.html) and it reuses the same
> code. For example `-fcuda-is-device` flag needs to be checked for [legacy HIP
> host
>
@@ -333,6 +333,9 @@ void Flang::AddAMDGPUTargetArgs(const ArgList ,
StringRef Val = A->getValue();
CmdArgs.push_back(Args.MakeArgString("-mcode-object-version=" + Val));
}
+
+ const ToolChain = getToolChain();
+ TC.addClangTargetOptions(Args, CmdArgs,
@@ -333,6 +333,9 @@ void Flang::AddAMDGPUTargetArgs(const ArgList ,
StringRef Val = A->getValue();
CmdArgs.push_back(Args.MakeArgString("-mcode-object-version=" + Val));
}
+
+ const ToolChain = getToolChain();
+ TC.addClangTargetOptions(Args, CmdArgs,
https://github.com/banach-space edited
https://github.com/llvm/llvm-project/pull/96742
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/banach-space commented:
> fcuda-is-device flag is not used by Flang currently. In the future it will be
> needed for Flang equivalent functions:
> AMDGPUTargetCodeGenInfo::getGlobalVarAddressSpace
> AMDGPUTargetInfo::getTargetDefines .
I don't follow - why would anything
https://github.com/banach-space approved this pull request.
https://github.com/llvm/llvm-project/pull/96678
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
banach-space wrote:
> LLVM Buildbot has detected a new failure on builder `flang-aarch64-libcxx`
> running on `linaro-flang-aarch64-libcxx` while building `clang,flang,mlir` at
> step 6 "test-build-unified-tree-check-flang".
>
> Full details are available at:
>
https://github.com/banach-space closed
https://github.com/llvm/llvm-project/pull/95043
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
banach-space wrote:
> if I do so should I also move the target-features-*.f90 tests?
Yes, but to me that would qualify as an "unrelated change" (i.e. sth for a
separate PR, no need to worry about it here).
In general, this PR is about enabling a flag in
https://github.com/banach-space closed
https://github.com/llvm/llvm-project/pull/95708
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/banach-space edited
https://github.com/llvm/llvm-project/pull/95708
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/banach-space approved this pull request.
LGTM, thanks for implementing this
https://github.com/llvm/llvm-project/pull/95043
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
banach-space wrote:
This is probably a border-line case, but I would consider this a "driver"
rather than a "lowering" test. I'm biased though
https://github.com/llvm/llvm-project/pull/95043
___
cfe-commits
banach-space wrote:
> You also need to remove the use of this pass (for testing purposes) from
> clang-tools-extra.
Done, thanks!
I don't understand why CI still fails:
```bash
Total Discovered Tests: 60135
--
| Skipped :15 (0.02%)
| Unsupported : 1005 (1.67%)
|
https://github.com/banach-space updated
https://github.com/llvm/llvm-project/pull/95708
From d75a05030447e8bcb1dd0b575ff5e7aa5c89f0bb Mon Sep 17 00:00:00 2001
From: Andrzej Warzynski
Date: Sun, 16 Jun 2024 13:58:41 +0100
Subject: [PATCH 1/3] [llvm] Remove the Legacy PM Hello example
The
@@ -0,0 +1,16 @@
+! RUN: %flang_fc1 -triple aarch64 -emit-llvm -mcmodel=tiny %s -o - | FileCheck
%s -check-prefix=CHECK-TINY
banach-space wrote:
[nit] IMHO, using `CHECK` for prefixes in a test with multiple prefixes is just
noise. Why not drop `CHECK`?
https://github.com/banach-space approved this pull request.
The high-level stuff makes sense to me, thanks! I'm not familiar with
`-mcmodel`, so can't tell for sure whether the tests are correct Ideally one
more person would take a look - @tblah or @pawosm-arm ?
https://github.com/banach-space edited
https://github.com/llvm/llvm-project/pull/95411
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -2823,3 +2823,84 @@ void tools::addOffloadCompressArgs(const
llvm::opt::ArgList ,
CmdArgs.push_back(
TCArgs.MakeArgString(Twine("-compression-level=") + Arg->getValue()));
}
+
+void tools::addMCModel(const Driver , const llvm::opt::ArgList ,
https://github.com/banach-space approved this pull request.
https://github.com/llvm/llvm-project/pull/94763
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -32,6 +32,9 @@ class TargetOptions {
/// If given, the name of the target CPU to generate code for.
std::string cpu;
+ /// If given, the name of the target CPU to tune code for.
+ std::string tuneCPU;
banach-space wrote:
I'm fine with longer
https://github.com/banach-space commented:
Thanks for working on this @AlexisPerry ! Could you add some tests?
https://github.com/llvm/llvm-project/pull/95043
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/banach-space edited
https://github.com/llvm/llvm-project/pull/95043
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1146,6 +1150,54 @@ void CodeGenAction::embedOffloadObjects() {
}
}
+void CodeGenAction::linkBuiltinBCLibs() {
banach-space wrote:
+1
https://github.com/llvm/llvm-project/pull/94763
___
cfe-commits mailing
@@ -0,0 +1,18 @@
+
+!--
+! RUN lines
+!--
+! Embed something that can be easily checked
+! RUN: %flang_fc1 -emit-llvm -triple x86_64-unknown-linux-gnu -o -
-mlink-builtin-bitcode %S/Inputs/bclib.bc %s 2>&1 | FileCheck %s
+
+! CHECK: define internal void @libfun_
@@ -0,0 +1,18 @@
+
+!--
+! RUN lines
+!--
+! Embed something that can be easily checked
+! RUN: %flang_fc1 -emit-llvm -triple x86_64-unknown-linux-gnu -o -
-mlink-builtin-bitcode %S/Inputs/bclib.bc %s 2>&1 | FileCheck %s
banach-space wrote:
1.
@@ -0,0 +1,18 @@
+
+!--
+! RUN lines
+!--
banach-space wrote:
In the past, folks asked not to add such comments in tests. Let's stick with
that.
https://github.com/llvm/llvm-project/pull/94763
___
@@ -0,0 +1,18 @@
+
+!--
+! RUN lines
+!--
+! Embed something that can be easily checked
+! RUN: %flang_fc1 -emit-llvm -triple x86_64-unknown-linux-gnu -o -
-mlink-builtin-bitcode %S/Inputs/bclib.bc %s 2>&1 | FileCheck %s
+
+! CHECK: define internal void @libfun_
https://github.com/banach-space approved this pull request.
https://github.com/llvm/llvm-project/pull/94359
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
banach-space wrote:
Should be fixed by https://github.com/llvm/llvm-project/pull/93794
https://github.com/llvm/llvm-project/pull/92338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
banach-space wrote:
Hi, thanks for this contribution. Sadly, it messes up my local checkout on
MacOS (which is insensitive when it comes to files names). These files are
problematic:
* "asm-constraint-jR.ll" and "asm-constraint-jr.ll"
Please, could you rename them so that they are not
banach-space wrote:
> I've left the flang test as the flang directory doesn't change with
> `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` and removed the other test. I hope this
> is what you meant @MaskRay
That's matches how I read that suggestion re
`LLVM_ENABLE_PER_TARGET_RUNTIME_DIR`, thanks!
https://github.com/banach-space approved this pull request.
LGTM, thanks!
https://github.com/llvm/llvm-project/pull/91660
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
banach-space wrote:
Thank you for this contribution Paul!
> Many autotools-utilizing projects (mpich among them) fail to complete the
> configure step since it tries to invoke the (unknown to them) Fortran
> compiler always with the -fallow-argument-mismatch flag.
It sounds like an issue
banach-space wrote:
[nit] Once you have more than one prefix, `CHECK` becomes noise. At least IMHO
(we all know that these are "check" lines). You could use more descriptive
prefixes instead, e.g. `CHECK-PORT` -> `PORTABILITY` (it wasn't obvious to me
https://github.com/banach-space approved this pull request.
LGTM, thanks!
https://github.com/llvm/llvm-project/pull/90420
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/banach-space edited
https://github.com/llvm/llvm-project/pull/90420
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/banach-space approved this pull request.
LGTM, thanks for working on this and for addressing my comments
https://github.com/llvm/llvm-project/pull/90886
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
@@ -0,0 +1,8 @@
+! REQUIRES: system-windows
+!
+! RUN: %clang --driver-mode=flang -### %s -Ltest 2>&1 | FileCheck %s
banach-space wrote:
[nit] `test` -> `random_test_dir` or something else that will make this stand
out a bit more (makes parsing tests a bit
@@ -0,0 +1,8 @@
+! REQUIRES: system-windows
+!
+! RUN: %clang --driver-mode=flang -### %s -Ltest 2>&1 | FileCheck %s
+!
+! Test that user provided paths come before the Flang runtimes and compiler-rt
+! CHECK: "-libpath:test"
banach-space wrote:
Wondering how to
https://github.com/banach-space approved this pull request.
LGTM, thanks!
I've left some nits, feel free to ignore
https://github.com/llvm/llvm-project/pull/90758
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/banach-space edited
https://github.com/llvm/llvm-project/pull/90758
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
banach-space wrote:
> > How does this compare to GFortran and Classic Flang? Anything resembling
> > this flag?
>
> GFortran does not have it, but Classic Flang does. So, it's closing a gap to
> Classic Flang here as well.
Do you know the meaning for Classic Flang? Would be great to make
@@ -0,0 +1,3 @@
+! RUN: %flang -print-resource-dir -resource-dir=%S/Inputs/resource_dir \
+! RUN: | FileCheck -check-prefix=PRINT-RESOURCE-DIR
-DFILE=%S/Inputs/resource_dir %s
banach-space wrote:
> So, my feeling is that this refactoring should take place in a
@@ -5474,7 +5474,7 @@ def print_prog_name_EQ : Joined<["-", "--"],
"print-prog-name=">,
Visibility<[ClangOption, CLOption]>;
def print_resource_dir : Flag<["-", "--"], "print-resource-dir">,
HelpText<"Print the resource directory pathname">,
- Visibility<[ClangOption,
@@ -5474,7 +5474,7 @@ def print_prog_name_EQ : Joined<["-", "--"],
"print-prog-name=">,
Visibility<[ClangOption, CLOption]>;
def print_resource_dir : Flag<["-", "--"], "print-resource-dir">,
HelpText<"Print the resource directory pathname">,
- Visibility<[ClangOption,
@@ -0,0 +1,3 @@
+! RUN: %flang -print-resource-dir -resource-dir=%S/Inputs/resource_dir \
+! RUN: | FileCheck -check-prefix=PRINT-RESOURCE-DIR
-DFILE=%S/Inputs/resource_dir %s
+! PRINT-RESOURCE-DIR: [[FILE]]
banach-space wrote:
I have a suspicion that this
@@ -0,0 +1,3 @@
+! RUN: %flang -print-resource-dir -resource-dir=%S/Inputs/resource_dir \
+! RUN: | FileCheck -check-prefix=PRINT-RESOURCE-DIR
-DFILE=%S/Inputs/resource_dir %s
banach-space wrote:
You should be able to avoid repeating "%S/Inputs/resource_dir"
@@ -250,6 +247,25 @@ void Driver::setDriverMode(StringRef Value) {
Diag(diag::err_drv_unsupported_option_argument) << OptName << Value;
}
+void Driver::setResourceDirectory() {
+ // Compute the path to the resource directory, depending on the driver mode.
+ switch
banach-space wrote:
> > What's the definition of "resource dir" for Fortran?
>
> I'd like to at least have it point to where the MODULE files live.
>
> When I look at what clang emits, then Flang's resource directory should
> rather point to the place, where Flang has its `lib` and `include`
https://github.com/banach-space commented:
What's the definition of "resource dir" for Fortran?
https://github.com/llvm/llvm-project/pull/90886
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
banach-space wrote:
Great work David, thanks! Could you add some documentation explaining _where_
`main` would be coming from in the case of mixed-source compilation? In fact,
is that tested anywhere?
Also, IMHO it would be good to advertise this on Discourse (thinking
specifically about
https://github.com/banach-space approved this pull request.
https://github.com/llvm/llvm-project/pull/88932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
banach-space wrote:
> Would you like me to introduce DocBriefForVariants?
+1 That would be helpful for `-I`:
*
https://flang.llvm.org/docs/FlangCommandLineReference.html#cmdoption-flang-I-dir
Ideally we'd find more examples (so that you are not adding it for just one
option).
As for this:
banach-space wrote:
> > Clang is also mentioned for the diagnostic warnings reference, which mostly
> > applies to C/C++/Obj-C, not Fortran. #81726 already tried to fix this, and
> > I don't know a better solution.
>
> Do you mean that this PR fixes this, or that you noticed this problem
https://github.com/banach-space approved this pull request.
Nice, thank you! LGTM
https://github.com/llvm/llvm-project/pull/88932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/banach-space approved this pull request.
LGTM, thanks for the updates!
https://github.com/llvm/llvm-project/pull/84944
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
@@ -0,0 +1,13 @@
+! Test -fcuda option
+! RUN: %flang -fc1 -cpp -fcuda -fdebug-unparse %s -o - | FileCheck %s
banach-space wrote:
Could you add a RUN line without enabling CUDA? Otherwise it's hard to see
what's being tested and what the impact of enabling CUDA
@@ -6488,6 +6488,9 @@ defm stack_arrays : BoolOptionWithoutMarshalling<"f",
"stack-arrays",
defm loop_versioning : BoolOptionWithoutMarshalling<"f",
"version-loops-for-stride",
PosFlag,
NegFlag>;
+
+def fcuda : Flag<["-"], "fcuda">, Group,
banach-space
https://github.com/banach-space edited
https://github.com/llvm/llvm-project/pull/81869
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/banach-space commented:
+1
I've been thinking about something similar for a while now, but I've never had
the cycles to try implementing it. Thanks for working on this! The OpenMP
option is an excellent example for "why" to do this. I don't expect _that many_
options
https://github.com/banach-space edited
https://github.com/llvm/llvm-project/pull/81869
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -3382,10 +3382,19 @@ def fopenmp : Flag<["-"], "fopenmp">, Group,
HelpText<"Parse OpenMP pragmas and generate parallel code.">;
def fno_openmp : Flag<["-"], "fno-openmp">, Group,
Flags<[NoArgumentUnused]>;
+class OpenMPVersionHelp {
+ string str = !strconcat(
+"Set
@@ -93,6 +93,11 @@ class OptionGroup {
// Define the option class.
+class HelpTextForVisibility {
banach-space wrote:
Naming is hard :) How about `HelpTextVariant`? Basically,
`HelpTextForVisibility` refers to the implementation detail ("visibility") and
https://github.com/banach-space approved this pull request.
I've been dreaming to have a fix for this for ages, thank you! This is along
the lines of what I had in mind, thanks for implementing it!
LGTM
https://github.com/llvm/llvm-project/pull/81726
https://github.com/banach-space approved this pull request.
Nice, thank you!
https://github.com/llvm/llvm-project/pull/81490
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/banach-space edited
https://github.com/llvm/llvm-project/pull/81490
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -0,0 +1,13 @@
+// RUN: %flang -target x86_64-unknown-linux -masm=intel -S %s -### 2>&1 |
FileCheck --check-prefix=CHECK-INTEL %s
banach-space wrote:
1. Use Fortran style comments :)
2. Missing requires?
https://github.com/llvm/llvm-project/pull/81490
banach-space wrote:
Thank you for your continued effort to improve this!
> Anyone knows if this is expected and Visibility should be explicitly
> specified in each alias, to add support to flang-new, or is this an issue and
> the alias should have the same visibility as the original option?
banach-space wrote:
> Not that it's the end of the world if this doesn't get in before the this
> release, but @sscalpone , exactly what feedback are we wanting before merging?
IIUC, this:
https://discourse.llvm.org/t/proposal-rename-flang-new-to-flang/69462/55
@@ -1193,6 +1193,16 @@ static void addFortranMain(const ToolChain , const
ArgList ,
return;
}
+ const Driver = TC.getDriver();
+ const char *LinkFlag = "-lFortran_main";
+
+ // warn if -lFortran_main was already specified
+ for (const char *arg : CmdArgs) {
+
@@ -1193,6 +1193,16 @@ static void addFortranMain(const ToolChain , const
ArgList ,
return;
}
+ const Driver = TC.getDriver();
banach-space wrote:
Move this whole block below the `// 2. GNU and similar`? Otherwise it seems
that this is still part
@@ -1193,6 +1193,16 @@ static void addFortranMain(const ToolChain , const
ArgList ,
return;
}
+ const Driver = TC.getDriver();
+ const char *LinkFlag = "-lFortran_main";
+
+ // warn if -lFortran_main was already specified
banach-space wrote:
@@ -1193,6 +1193,16 @@ static void addFortranMain(const ToolChain , const
ArgList ,
return;
}
+ const Driver = TC.getDriver();
+ const char *LinkFlag = "-lFortran_main";
banach-space wrote:
```suggestion
const char *FortranMainLinkFlag =
https://github.com/banach-space edited
https://github.com/llvm/llvm-project/pull/79016
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/banach-space approved this pull request.
> Intended to warn users of the 19.x release not to do this.
>
> A better solution should be found for the 20.x release. See discussion in
> https://github.com/llvm/llvm-project/pull/78152.
>
> Unfortunately there is no warning on
banach-space wrote:
> @banach-space I tentatively support this. But I am concerned that disallowing
> `-lFortran_main` and renaming the library will break some existing (badly
> configured) builds (this is not just theoretical - see my comments above).
> Maybe we should merge a deprecation
@@ -354,6 +354,27 @@ void Flang::addTargetOptions(const ArgList ,
CmdArgs.push_back(Args.MakeArgString(CPU));
}
+ if (Arg *A = Args.getLastArg(options::OPT_moutline_atomics,
+ options::OPT_mno_outline_atomics)) {
+// Option
banach-space wrote:
> I don't know what is the right way to handle the case that users have
> conflicting flags specified.
This comment made me realise what might be the source of confusion/friction.
With `-fno-fortran-main` and `-lFortran_main`, there are two very different
mechanisms to
banach-space wrote:
> > Can you remind me the benefits of using `-isysroot` over `-sysroot` to
> > begin with? I think that switching to `-sysroot` is fine, but I also want
> > to make sure we're not missing anything.
>
> It seems to me that Apple prefers to use `-isysroot` to select the SDK:
banach-space wrote:
Thank you for this summary, @luporl !
> I think `-isysroot` should have preference over `DEFAULT_SYSROOT`, but:
>
> * I don't have much knowledge about the driver.
> * Since this has been the behavior for a long time, I fear that changing it
> may break some use case.
> *
banach-space wrote:
I am not too fond of this.
The current design has taken quite some effort and a few discussions to land in
its current form. Also, Flang has gained proper documentation for this:
*
https://github.com/llvm/llvm-project/blob/main/flang/docs/FlangDriver.md#linker-driver
https://github.com/banach-space approved this pull request.
LGTM, thanks!
> This has only be tested on x86 Linux.
[nit] Note that the test that you've added will run on any platform. Unless
that's referring to something else?
https://github.com/llvm/llvm-project/pull/77360
banach-space wrote:
> > IIUC, this means that on older system compilation will indeed fail without
> > `-pthread`, but shouldn't be needed on newer systems. @tarunprabhu -
> > perhaps add a link to that article in your test and add a note that on many
> > systems compilation will succeed even
1 - 100 of 309 matches
Mail list logo