[PATCH] D114023: [Driver] Pass --fix-cortex-a53-843419 automatically on Fuchsia
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7f0e741db97c: [Driver] Pass --fix-cortex-a53-843419
automatically on Fuchsia (authored by phosek).
Herald added a subscriber: MaskRay.
Herald added a project: All.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114023/new/
https://reviews.llvm.org/D114023
Files:
clang/lib/Driver/ToolChains/Fuchsia.cpp
clang/test/Driver/fuchsia.c
Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -41,6 +41,7 @@
// CHECK: "-pie"
// CHECK: "--build-id"
// CHECK: "--hash-style=gnu"
+// CHECK-AARCH64: "--fix-cortex-a53-843419"
// CHECK: "-dynamic-linker" "ld.so.1"
// CHECK: Scrt1.o
// CHECK-NOT: crti.o
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -37,6 +37,8 @@
static_cast(getToolChain());
const Driver = ToolChain.getDriver();
+ const llvm::Triple = ToolChain.getEffectiveTriple();
+
ArgStringList CmdArgs;
// Silence warning for "clang -g foo.o -o foo"
@@ -84,6 +86,12 @@
CmdArgs.push_back("--hash-style=gnu");
}
+ if (ToolChain.getArch() == llvm::Triple::aarch64) {
+std::string CPU = getCPUName(D, Args, Triple);
+if (CPU.empty() || CPU == "generic" || CPU == "cortex-a53")
+ CmdArgs.push_back("--fix-cortex-a53-843419");
+ }
+
CmdArgs.push_back("--eh-frame-hdr");
if (Args.hasArg(options::OPT_static))
Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -41,6 +41,7 @@
// CHECK: "-pie"
// CHECK: "--build-id"
// CHECK: "--hash-style=gnu"
+// CHECK-AARCH64: "--fix-cortex-a53-843419"
// CHECK: "-dynamic-linker" "ld.so.1"
// CHECK: Scrt1.o
// CHECK-NOT: crti.o
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -37,6 +37,8 @@
static_cast(getToolChain());
const Driver = ToolChain.getDriver();
+ const llvm::Triple = ToolChain.getEffectiveTriple();
+
ArgStringList CmdArgs;
// Silence warning for "clang -g foo.o -o foo"
@@ -84,6 +86,12 @@
CmdArgs.push_back("--hash-style=gnu");
}
+ if (ToolChain.getArch() == llvm::Triple::aarch64) {
+std::string CPU = getCPUName(D, Args, Triple);
+if (CPU.empty() || CPU == "generic" || CPU == "cortex-a53")
+ CmdArgs.push_back("--fix-cortex-a53-843419");
+ }
+
CmdArgs.push_back("--eh-frame-hdr");
if (Args.hasArg(options::OPT_static))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D114023: [Driver] Pass --fix-cortex-a53-843419 automatically on Fuchsia
mcgrathr accepted this revision.
mcgrathr added inline comments.
This revision is now accepted and ready to land.
Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:91
+std::string CPU = getCPUName(D, Args, Triple);
+if (CPU.empty() || CPU == "generic" || CPU == "cortex-a53")
+ CmdArgs.push_back("--fix-cortex-a53-843419");
phosek wrote:
> mcgrathr wrote:
> > How does this relate to -march, -mtune, and/or -mcpu?
> > It's not at all clear to me how we discern automatically when a A53 might
> > be included in the supported target CPU set. I'm not entirely sure we
> > should have automatic injection in some cases where A53 is a valid target
> > but not all. That is, if some compiler option combinations that produce
> > code meant to be compatible with an A53 won't automatically get the --fix
> > option, then perhaps it's better to require the explicit --fix option when
> > supporting A53 is the explicit intent.
> This is the same logic as used by the [Gnu
> driver](https://github.com/llvm/llvm-project/blob/7a7c059d867554e116244ad5639d05d75ed1a7cd/clang/lib/Driver/ToolChains/Gnu.cpp#L451).
> `getCPUName` only considers `-mcpu`, it ignores `-march` and `-mtune` as far
> as I can tell.
If the empty or "generic" case is what will apply when no `-mcpu` switch is
given regardless of `-march` then I guess this is adequate since we're not
really concerned with any architectures older than A53 where someone might use
`-mcpu=...` as a proxy for "X and later" where Xhttps://reviews.llvm.org/D114023/new/
https://reviews.llvm.org/D114023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D114023: [Driver] Pass --fix-cortex-a53-843419 automatically on Fuchsia
phosek added inline comments.
Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:91
+std::string CPU = getCPUName(D, Args, Triple);
+if (CPU.empty() || CPU == "generic" || CPU == "cortex-a53")
+ CmdArgs.push_back("--fix-cortex-a53-843419");
mcgrathr wrote:
> How does this relate to -march, -mtune, and/or -mcpu?
> It's not at all clear to me how we discern automatically when a A53 might be
> included in the supported target CPU set. I'm not entirely sure we should
> have automatic injection in some cases where A53 is a valid target but not
> all. That is, if some compiler option combinations that produce code meant
> to be compatible with an A53 won't automatically get the --fix option, then
> perhaps it's better to require the explicit --fix option when supporting A53
> is the explicit intent.
This is the same logic as used by the [Gnu
driver](https://github.com/llvm/llvm-project/blob/7a7c059d867554e116244ad5639d05d75ed1a7cd/clang/lib/Driver/ToolChains/Gnu.cpp#L451).
`getCPUName` only considers `-mcpu`, it ignores `-march` and `-mtune` as far
as I can tell.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114023/new/
https://reviews.llvm.org/D114023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D114023: [Driver] Pass --fix-cortex-a53-843419 automatically on Fuchsia
mcgrathr added inline comments.
Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:91
+std::string CPU = getCPUName(D, Args, Triple);
+if (CPU.empty() || CPU == "generic" || CPU == "cortex-a53")
+ CmdArgs.push_back("--fix-cortex-a53-843419");
How does this relate to -march, -mtune, and/or -mcpu?
It's not at all clear to me how we discern automatically when a A53 might be
included in the supported target CPU set. I'm not entirely sure we should have
automatic injection in some cases where A53 is a valid target but not all.
That is, if some compiler option combinations that produce code meant to be
compatible with an A53 won't automatically get the --fix option, then perhaps
it's better to require the explicit --fix option when supporting A53 is the
explicit intent.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114023/new/
https://reviews.llvm.org/D114023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D114023: [Driver] Pass --fix-cortex-a53-843419 automatically on Fuchsia
phosek created this revision.
phosek added reviewers: mcgrathr, leonardchan.
Herald added a subscriber: abrachet.
phosek requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
When targeting cortex-a53, set this linker flag rather than relying
on the toolchain users to do it in their build.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D114023
Files:
clang/lib/Driver/ToolChains/Fuchsia.cpp
clang/test/Driver/fuchsia.c
Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -40,6 +40,7 @@
// CHECK: "-pie"
// CHECK: "--build-id"
// CHECK: "--hash-style=gnu"
+// CHECK-AARCH64: "--fix-cortex-a53-843419"
// CHECK: "-dynamic-linker" "ld.so.1"
// CHECK: Scrt1.o
// CHECK-NOT: crti.o
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -37,6 +37,8 @@
static_cast(getToolChain());
const Driver = ToolChain.getDriver();
+ const llvm::Triple = ToolChain.getEffectiveTriple();
+
ArgStringList CmdArgs;
// Silence warning for "clang -g foo.o -o foo"
@@ -84,6 +86,12 @@
CmdArgs.push_back("--hash-style=gnu");
}
+ if (ToolChain.getArch() == llvm::Triple::aarch64) {
+std::string CPU = getCPUName(D, Args, Triple);
+if (CPU.empty() || CPU == "generic" || CPU == "cortex-a53")
+ CmdArgs.push_back("--fix-cortex-a53-843419");
+ }
+
CmdArgs.push_back("--eh-frame-hdr");
if (Args.hasArg(options::OPT_static))
Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -40,6 +40,7 @@
// CHECK: "-pie"
// CHECK: "--build-id"
// CHECK: "--hash-style=gnu"
+// CHECK-AARCH64: "--fix-cortex-a53-843419"
// CHECK: "-dynamic-linker" "ld.so.1"
// CHECK: Scrt1.o
// CHECK-NOT: crti.o
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -37,6 +37,8 @@
static_cast(getToolChain());
const Driver = ToolChain.getDriver();
+ const llvm::Triple = ToolChain.getEffectiveTriple();
+
ArgStringList CmdArgs;
// Silence warning for "clang -g foo.o -o foo"
@@ -84,6 +86,12 @@
CmdArgs.push_back("--hash-style=gnu");
}
+ if (ToolChain.getArch() == llvm::Triple::aarch64) {
+std::string CPU = getCPUName(D, Args, Triple);
+if (CPU.empty() || CPU == "generic" || CPU == "cortex-a53")
+ CmdArgs.push_back("--fix-cortex-a53-843419");
+ }
+
CmdArgs.push_back("--eh-frame-hdr");
if (Args.hasArg(options::OPT_static))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
