[PATCH] D159398: [AArch64][Clang] Disable out-of-line atomics in ffreestanding env
yota9 created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
yota9 requested review of this revision.
Herald added subscribers: cfe-commits, wangpc, MaskRay.
Herald added a project: clang.
In freestanding environment we don't want extra dependencies on the helpers
that implements atomic operations. So don't enable out-of-line atomics in
this situation.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D159398
Files:
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/aarch64-features.c
Index: clang/test/Driver/aarch64-features.c
===
--- clang/test/Driver/aarch64-features.c
+++ clang/test/Driver/aarch64-features.c
@@ -32,6 +32,12 @@
// RUN: %clang --target=arm64-unknown-linux -rtlib=compiler-rt \
// RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-OUTLINE-ATOMICS-ON %s
+// RUN: %clang --target=aarch64-linux-gnu -rtlib=compiler-rt -ffreestanding \
+// RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-OUTLINE-ATOMICS-OFF %s
+
+// RUN: %clang --target=arm64-unknown-linux -rtlib=compiler-rt -ffreestanding \
+// RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-OUTLINE-ATOMICS-OFF %s
+
// RUN: %clang --target=aarch64 -rtlib=compiler-rt \
// RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-OUTLINE-ATOMICS-OFF %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7494,7 +7494,7 @@
CmdArgs.push_back("-outline-atomics");
}
}
- } else if (Triple.isAArch64() &&
+ } else if (!Freestanding && Triple.isAArch64() &&
getToolChain().IsAArch64OutlineAtomicsDefault(Args)) {
CmdArgs.push_back("-target-feature");
CmdArgs.push_back("+outline-atomics");
Index: clang/test/Driver/aarch64-features.c
===
--- clang/test/Driver/aarch64-features.c
+++ clang/test/Driver/aarch64-features.c
@@ -32,6 +32,12 @@
// RUN: %clang --target=arm64-unknown-linux -rtlib=compiler-rt \
// RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-OUTLINE-ATOMICS-ON %s
+// RUN: %clang --target=aarch64-linux-gnu -rtlib=compiler-rt -ffreestanding \
+// RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-OUTLINE-ATOMICS-OFF %s
+
+// RUN: %clang --target=arm64-unknown-linux -rtlib=compiler-rt -ffreestanding \
+// RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-OUTLINE-ATOMICS-OFF %s
+
// RUN: %clang --target=aarch64 -rtlib=compiler-rt \
// RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-OUTLINE-ATOMICS-OFF %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7494,7 +7494,7 @@
CmdArgs.push_back("-outline-atomics");
}
}
- } else if (Triple.isAArch64() &&
+ } else if (!Freestanding && Triple.isAArch64() &&
getToolChain().IsAArch64OutlineAtomicsDefault(Args)) {
CmdArgs.push_back("-target-feature");
CmdArgs.push_back("+outline-atomics");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D159398: [AArch64][Clang] Disable out-of-line atomics in ffreestanding env
yota9 added a comment. The build failure has nothing to do with the patch and it caused by https://reviews.llvm.org/D140612 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159398/new/ https://reviews.llvm.org/D159398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D159398: [AArch64][Clang] Disable outline atomics in freestanding env
yota9 added a comment. The problem here is that if you would remove the freestanding condition and run this test it would fail since "-target-feature" "+outline-atomics" would be passed to the clang. And usually we don't want the compiler to use any external symbols in freestanding mode, this is the reason why -fno-builtin is implied with freestanding option. So it seems to be logical to me to disable outline-atomics too in this situation even if the compiler have runtime library available Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159398/new/ https://reviews.llvm.org/D159398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D159398: [AArch64][Clang] Disable outline atomics in freestanding env
yota9 added a comment. >> Outline atomics are dependent on runtime library availability ( libgcc or >> compler-rt ). I understand that, but we can use compiler that have runtime library, but in freestanding mode usually we don't want it to be used. I understand that you could disable it with extra option and that for now it would be different with gcc, but it looks debatable to me that such a behaviour in gcc is correct and expected, maybe someone need to change it there too. (Please keep in mind that I might be wrong with my position and during discussion please consider this patch as NFC so we could discuss here am I right or not :)). My point is that the gcc is not a golden standard in this questions too and in my mind such a behaviour (disabling extra dependencies in freestanding mode without passing extra flags) would be expected the very same way the -fno-builtint get's auto implied with -ffreestanding flag passed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159398/new/ https://reviews.llvm.org/D159398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D159398: [AArch64][Clang] Disable outline atomics in freestanding env
yota9 added a comment. Great! I've add MaskRay to the reviewers list as known expert in both clang and gcc, maybe he have some thoughts on this proposal :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159398/new/ https://reviews.llvm.org/D159398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D159398: [AArch64][Clang] Disable outline atomics in freestanding env
yota9 added a comment. Also please keep in mind that despite of different behaviour in gcc/clang not implying outline atomics won't result in any problems, but implying them in cases where we don't want them might result in some problems. Offtopic: Outlining atomics seems to be very CPU specific thing. In my experience LSE were ~= old exclusive semantics. So adding extra call + extra bit check (too bad IFUNCs are not used :)) each time it would be executed seems to be quite an extra load (for CPU, TLB, dcache..), so I'm not sure that outline atomics is a win-win thing (at least on some of the CPUs). This is absolutely not a case for this patch anyway, just some of my thoughts, I would be glad to hear other opinions :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159398/new/ https://reviews.llvm.org/D159398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
