[PATCH] D93579: [clang][AVR] Improve avr-ld command line options
benshi001 created this revision.
benshi001 added reviewers: dylanmckay, aykevl.
Herald added a subscriber: Jim.
benshi001 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D93579
Files:
clang/lib/Driver/ToolChains/AVR.cpp
clang/test/Driver/avr-ld.c
Index: clang/test/Driver/avr-ld.c
===
--- /dev/null
+++ clang/test/Driver/avr-ld.c
@@ -0,0 +1,46 @@
+// RUN: %clang -### --target=avr -mmcu=at90s2313 %s 2>&1 | FileCheck -check-prefix LINKA %s
+// LINKA: {{".*ld.*"}} {{.*}} {{"-L.*tiny-stack"}} {{.*}} "-Tdata=0x800060" {{.*}} "-lat90s2313" "-mavr2"
+
+// RUN: %clang -### --target=avr -mmcu=at90s8515 %s 2>&1 | FileCheck -check-prefix LINKB %s
+// LINKB: {{".*ld.*"}} {{.*}} "-Tdata=0x800060" {{.*}} "-lat90s8515" "-mavr2"
+
+// RUN: %clang -### --target=avr -mmcu=attiny13 %s 2>&1 | FileCheck -check-prefix LINKC %s
+// LINKC: {{".*ld.*"}} {{.*}} {{"-L.*avr25/tiny-stack"}} {{.*}} "-Tdata=0x800060" {{.*}} "-lattiny13" "-mavr25"
+
+// RUN: %clang -### --target=avr -mmcu=attiny44 %s 2>&1 | FileCheck -check-prefix LINKD %s
+// LINKD: {{".*ld.*"}} {{.*}} {{"-L.*avr25"}} {{.*}} "-Tdata=0x800060" {{.*}} "-lattiny44" "-mavr25"
+
+// RUN: %clang -### --target=avr -mmcu=atmega103 %s 2>&1 | FileCheck -check-prefix LINKE %s
+// LINKE: {{".*ld.*"}} {{.*}} {{"-L.*avr31"}} {{.*}} "-Tdata=0x800060" {{.*}} "-latmega103" "-mavr31"
+
+// RUN: %clang -### --target=avr -mmcu=atmega8u2 %s 2>&1 | FileCheck -check-prefix LINKF %s
+// LINKF: {{".*ld.*"}} {{.*}} {{"-L.*avr35"}} {{.*}} "-Tdata=0x800100" {{.*}} "-latmega8u2" "-mavr35"
+
+// RUN: %clang -### --target=avr -mmcu=atmega48pa %s 2>&1 | FileCheck -check-prefix LINKG %s
+// LINKG: {{".*ld.*"}} {{.*}} {{"-L.*avr4"}} {{.*}} "-Tdata=0x800100" {{.*}} "-latmega48pa" "-mavr4"
+
+// RUN: %clang -### --target=avr -mmcu=atmega328 %s 2>&1 | FileCheck -check-prefix LINKH %s
+// LINKH: {{".*ld.*"}} {{.*}} {{"-L.*avr5"}} {{.*}} "-Tdata=0x800100" {{.*}} "-latmega328" "-mavr5"
+
+// RUN: %clang -### --target=avr -mmcu=atmega1281 %s 2>&1 | FileCheck -check-prefix LINKI %s
+// LINKI: {{".*ld.*"}} {{.*}} {{"-L.*avr51"}} {{.*}} "-Tdata=0x800200" {{.*}} "-latmega1281" "-mavr51"
+
+// RUN: %clang -### --target=avr -mmcu=atmega2560 %s 2>&1 | FileCheck -check-prefix LINKJ %s
+// LINKJ: {{".*ld.*"}} {{.*}} {{"-L.*avr6"}} {{.*}} "-Tdata=0x800200" {{.*}} "-latmega2560" "-mavr6"
+
+// RUN: %clang -### --target=avr -mmcu=attiny10 %s 2>&1 | FileCheck -check-prefix LINKK %s
+// LINKK: {{".*ld.*"}} {{.*}} {{"-L.*avrtiny"}} {{.*}} "-Tdata=0x800040" {{.*}} "-lattiny10" "-mavrtiny"
+
+// RUN: %clang -### --target=avr -mmcu=atxmega16a4 %s 2>&1 | FileCheck -check-prefix LINKL %s
+// LINKL: {{".*ld.*"}} {{.*}} {{"-L.*avrxmega2"}} {{.*}} "-Tdata=0x802000" {{.*}} "-latxmega16a4" "-mavrxmega2"
+
+// RUN: %clang -### --target=avr -mmcu=atxmega64b3 %s 2>&1 | FileCheck -check-prefix LINKM %s
+// LINKM: {{".*ld.*"}} {{.*}} {{"-L.*avrxmega4"}} {{.*}} "-Tdata=0x802000" {{.*}} "-latxmega64b3" "-mavrxmega4"
+
+// RUN: %clang -### --target=avr -mmcu=atxmega128a3u %s 2>&1 | FileCheck -check-prefix LINKN %s
+// LINKN: {{".*ld.*"}} {{.*}} {{"-L.*avrxmega6"}} {{.*}} "-Tdata=0x802000" {{.*}} "-latxmega128a3u" "-mavrxmega6"
+
+// RUN: %clang -### --target=avr -mmcu=atxmega128a1 %s 2>&1 | FileCheck -check-prefix LINKO %s
+// LINKO: {{".*ld.*"}} {{.*}} {{"-L.*avrxmega7"}} {{.*}} "-Tdata=0x802000" {{.*}} "-latxmega128a1" "-mavrxmega7"
+
+int main() { return 0; }
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -32,247 +32,248 @@
StringRef Name;
std::string SubPath;
StringRef Family;
+ unsigned DataAddr;
} MCUInfo[] = {
{"at90s1200", "", "avr1"},
{"attiny11", "", "avr1"},
{"attiny12", "", "avr1"},
{"attiny15", "", "avr1"},
{"attiny28", "", "avr1"},
-{"at90s2313", "tiny-stack", "avr2"},
-{"at90s2323", "tiny-stack", "avr2"},
-{"at90s2333", "tiny-stack", "avr2"},
-{"at90s2343", "tiny-stack", "avr2"},
-{"at90s4433", "tiny-stack", "avr2"},
-{"attiny22", "tiny-stack", "avr2"},
-{"attiny26", "tiny-stack", "avr2"},
-{"at90s4414", "", "avr2"},
-{"at90s4434", "", "avr2"},
-{"at90s8515", "", "avr2"},
-{"at90c8534", "", "avr2"},
-{"at90s8535", "", "avr2"},
-{"attiny13", "avr25/tiny-stack", "avr25"},
-{"attiny13a", "avr25/tiny-stack", "avr25"},
-{"attiny2313", "avr25/tiny-stack", "avr25"},
-{"attiny2313a", "avr25/tiny-stack", "avr25"},
-{"attiny24", "avr25/tiny-stack", "avr25"},
-{"attiny24a", "avr25/tiny-stack", "avr25"},
-{"attiny25", "avr25/tiny-stack", "avr25"},
-{"attiny261", "avr25/tiny-stack", "avr25"},
-{"attiny261a", "avr25/tiny-stack", "avr25"},
-{"at86rf401", "avr25", "avr25"},
-{"ata5272",
[PATCH] D93452: [clangd] Trim memory periodically
qchateau planned changes to this revision. qchateau marked 2 inline comments as done. qchateau added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:52 +// Malloc trim minimal period +std::chrono::seconds MemoryCleanupPeriod = std::chrono::seconds(60); sammccall wrote: > I don't particularly think the interval needs to be set through options > (memory profiling interval isn't), we can just hardcode this. > > On the other hand, doing this at all should be optional, and I think we > should enable it from ClangdMain - this is a weird thing for a library to be > doing by default, and as it stands we'll be doing it in unit tests and stuff. > > I think my favorite variant would be > ``` > /// If set, periodically called to release memory. > /// Consider llvm::sys::Process::ReleaseHeapMemory; > std::function MemoryCleanup = nullptr; > ``` Sounds good, do you think [1] we should throttle the calls to `MemoryCleanup` or [2] throttle the calls to `malloc_trim` from within the callback ? >I don't particularly think the interval needs to be set through options >(memory profiling interval isn't), we can just hardcode this. > >On the other hand, doing this at all should be optional, and I think we should >enable it from ClangdMain - this is a weird thing for a library to be doing by >default, and as it stands we'll be doing it in unit tests and stuff. I like the idea of configuring the `MemoryCleanup` function from ClangdMain, but it's as easy to have [3] an int option that configures the interval as it is to have [4] a boolean one to just disable it. I actually think both points are related: - If we choose [2], then I'd go for [3] because it is more configurable and adds no code complexity. - If we choose [1], then I agree we can go for [4] to avoid an extra option Comment at: llvm/lib/Support/Unix/Process.inc:122 +#if defined(HAVE_MALLOC_TRIM) + return malloc_trim(Pad); +#else sammccall wrote: > MaskRay wrote: > > If the user uses jemalloc/tcmalloc with glibc, malloc_trim may exist while > > the function may do nothing. > True. Unless I'm missing something, it won't be completely free - it'll still > acquire glibc-malloc locks and walk over whatever arenas exist. > > @MaskRay is there anything sensible to do about this? We can introduce a > cmake variable or something you're supposed to set if you're using another > allocator, but: > - that seems fragile > - fundamentally doesn't support switching allocators with LD_PRELOAD, I guess > I'm not worried about LD_PRELOAD, worst case it adds a little bit of extra runtime cost (calling malloc_trim although it's useless because we preloaded tcmalloc). But I agree there is an issue here: we detect which cleanup function to use depending on the headers (in cmake), but it should actually depend on what we link. Not sure if this can be done properly :/ For now we only want to cleanup on glibc malloc, but I'll investigate with tcmalloc and jemalloc, maybe they do not work so well either after all. Then the problem would be even worse: I'd be tempted to call a similar function for these libraries as well, and although the developer may have tcmalloc in its system include, it does not mean he is linking it -> undefined symbol Overall this approach of calling `malloc_trim` is looking grim to me because it depends on what's linked. Either we can continue in this way, or try to find another way around this issue: - Encourage the developer to link another malloc implementation, but then it's only useful if the various distributions do it - Continue to dig on why glibc malloc performs so bad with clangd and try to improve things in clangd code - Something else ? Comment at: llvm/lib/Support/Unix/Process.inc:124 +#else +#warning Cannot get malloc info on this platform + return false; sammccall wrote: > this is expected, we don't want a compiler warning here Same for windows I guess ? If I understand correctly your logic is "if it's not available that's because we don't need it: don't warn" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93452/new/ https://reviews.llvm.org/D93452 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 9c895ae - [ARM] Add clang command line support for -mharden-sls=
Author: Kristof Beyls
Date: 2020-12-19T12:49:26Z
New Revision: 9c895aea118a2f50ca8413372363c3ff6ecc21bf
URL:
https://github.com/llvm/llvm-project/commit/9c895aea118a2f50ca8413372363c3ff6ecc21bf
DIFF:
https://github.com/llvm/llvm-project/commit/9c895aea118a2f50ca8413372363c3ff6ecc21bf.diff
LOG: [ARM] Add clang command line support for -mharden-sls=
The command line syntax is identical to the -mharden-sls= command line
syntax for AArch64 targets.
Differential Revision: https://reviews.llvm.org/D93221
Added:
clang/test/Driver/sls-hardening-options.c
Modified:
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/lib/Driver/ToolChains/Arch/ARM.h
Removed:
clang/test/Driver/aarch64-sls-hardening-options.c
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index c67cce099a28..e92a4bf1dac5 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -348,6 +348,8 @@ def err_invalid_branch_protection: Error <
"invalid branch protection option '%0' in '%1'">;
def err_invalid_sls_hardening : Error<
"invalid sls hardening option '%0' in '%1'">;
+def err_sls_hardening_arm_not_supported : Error<
+ "-mharden-sls is only supported on armv7-a or later">;
def note_drv_command_failed_diag_msg : Note<
"diagnostic msg: %0">;
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 309a7298300f..ef590db1eecd 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -32,6 +32,12 @@ bool arm::isARMMProfile(const llvm::Triple &Triple) {
return llvm::ARM::parseArchProfile(Arch) == llvm::ARM::ProfileKind::M;
}
+// True if A-profile.
+bool arm::isARMAProfile(const llvm::Triple &Triple) {
+ llvm::StringRef Arch = Triple.getArchName();
+ return llvm::ARM::parseArchProfile(Arch) == llvm::ARM::ProfileKind::A;
+}
+
// Get Arch/CPU from args.
void arm::getARMArchCPUFromArgs(const ArgList &Args, llvm::StringRef &Arch,
llvm::StringRef &CPU, bool FromAs) {
@@ -606,6 +612,45 @@ void arm::getARMTargetFeatures(const Driver &D, const
llvm::Triple &Triple,
if (Args.hasArg(options::OPT_mno_neg_immediates))
Features.push_back("+no-neg-immediates");
+
+ // Enable/disable straight line speculation hardening.
+ if (Arg *A = Args.getLastArg(options::OPT_mharden_sls_EQ)) {
+StringRef Scope = A->getValue();
+bool EnableRetBr = false;
+bool EnableBlr = false;
+if (Scope != "none" && Scope != "all") {
+ SmallVector Opts;
+ Scope.split(Opts, ",");
+ for (auto Opt : Opts) {
+Opt = Opt.trim();
+if (Opt == "retbr") {
+ EnableRetBr = true;
+ continue;
+}
+if (Opt == "blr") {
+ EnableBlr = true;
+ continue;
+}
+D.Diag(diag::err_invalid_sls_hardening)
+<< Scope << A->getAsString(Args);
+break;
+ }
+} else if (Scope == "all") {
+ EnableRetBr = true;
+ EnableBlr = true;
+}
+
+if (EnableRetBr || EnableBlr)
+ if (!(isARMAProfile(Triple) && getARMSubArchVersionNumber(Triple) >= 7))
+D.Diag(diag::err_sls_hardening_arm_not_supported)
+<< Scope << A->getAsString(Args);
+
+if (EnableRetBr)
+ Features.push_back("+harden-sls-retbr");
+if (EnableBlr)
+ Features.push_back("+harden-sls-blr");
+ }
+
}
const std::string arm::getARMArch(StringRef Arch, const llvm::Triple &Triple) {
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.h
b/clang/lib/Driver/ToolChains/Arch/ARM.h
index 091c09b160ae..02d91cdaee13 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.h
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -63,6 +63,7 @@ void getARMTargetFeatures(const Driver &D, const llvm::Triple
&Triple,
std::vector &Features, bool ForAS);
int getARMSubArchVersionNumber(const llvm::Triple &Triple);
bool isARMMProfile(const llvm::Triple &Triple);
+bool isARMAProfile(const llvm::Triple &Triple);
} // end namespace arm
} // end namespace tools
diff --git a/clang/test/Driver/aarch64-sls-hardening-options.c
b/clang/test/Driver/aarch64-sls-hardening-options.c
deleted file mode 100644
index 250007aa1254..
--- a/clang/test/Driver/aarch64-sls-hardening-options.c
+++ /dev/null
@@ -1,45 +0,0 @@
-// Check the -mharden-sls= option, which has a required argument to select
-// scope.
-// RUN: %clang -target aarch64--none-eabi -c %s -### 2>&1 | \
-// RUN: FileCheck %s --check-prefix=RETBR-OFF --check-prefix=BLR-OFF
-
-// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=none 2>&1 | \
-// RUN: FileCheck %s --check-prefix=RETBR-OFF --check-prefix=BLR-OFF
-
-// RUN: %clang -target aarch64--none-eabi
[PATCH] D93221: [ARM] Add clang command line support for -mharden-sls=
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9c895aea118a: [ARM] Add clang command line support for
-mharden-sls= (authored by kristof.beyls).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93221/new/
https://reviews.llvm.org/D93221
Files:
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/lib/Driver/ToolChains/Arch/ARM.h
clang/test/Driver/aarch64-sls-hardening-options.c
clang/test/Driver/sls-hardening-options.c
Index: clang/test/Driver/sls-hardening-options.c
===
--- /dev/null
+++ clang/test/Driver/sls-hardening-options.c
@@ -0,0 +1,97 @@
+// Check the -mharden-sls= option, which has a required argument to select
+// scope.
+// RUN: %clang -target aarch64--none-eabi -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-OFF --check-prefix=BLR-OFF
+// RUN: %clang -target armv7a--none-eabi -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-OFF --check-prefix=BLR-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=none 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-OFF --check-prefix=BLR-OFF
+// RUN: %clang -target armv7a--none-eabi -c %s -### -mharden-sls=none 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-OFF --check-prefix=BLR-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=retbr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-ON --check-prefix=BLR-OFF
+// RUN: %clang -target armv7a--none-eabi -c %s -### -mharden-sls=retbr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-ON --check-prefix=BLR-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=blr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-OFF --check-prefix=BLR-ON
+// RUN: %clang -target armv7a--none-eabi -c %s -### -mharden-sls=blr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-OFF --check-prefix=BLR-ON
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=blr -mharden-sls=none 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-OFF --check-prefix=BLR-OFF
+// RUN: %clang -target armv7a--none-eabi -c %s -### -mharden-sls=blr -mharden-sls=none 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-OFF --check-prefix=BLR-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=blr -mharden-sls=retbr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-ON --check-prefix=BLR-OFF
+// RUN: %clang -target armv7a--none-eabi -c %s -### -mharden-sls=blr -mharden-sls=retbr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-ON --check-prefix=BLR-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=retbr,blr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-ON --check-prefix=BLR-ON
+// RUN: %clang -target armv7a--none-eabi -c %s -### -mharden-sls=retbr,blr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-ON --check-prefix=BLR-ON
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=all 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-ON --check-prefix=BLR-ON
+// RUN: %clang -target armv7a--none-eabi -c %s -### -mharden-sls=all 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-ON --check-prefix=BLR-ON
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=retbr,blr,retbr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-ON --check-prefix=BLR-ON
+// RUN: %clang -target armv7a--none-eabi -c %s -### -mharden-sls=retbr,blr,retbr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RETBR-ON --check-prefix=BLR-ON
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=retbr,blr,r 2>&1 | \
+// RUN: FileCheck %s --check-prefix=BAD-SLS-SPEC
+// RUN: %clang -target armv7a--none-eabi -c %s -### -mharden-sls=retbr,blr,r 2>&1 | \
+// RUN: FileCheck %s --check-prefix=BAD-SLS-SPEC
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=none,blr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=BAD-SLS-SPEC
+// RUN: %clang -target armv7a--none-eabi -c %s -### -mharden-sls=none,blr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=BAD-SLS-SPEC
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mharden-sls=all,-blr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=BAD-SLS-SPEC
+// RUN: %clang -target armv7a--none-eabi -c %s -### -mharden-sls=all,-blr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=BAD-SLS-SPEC
+
+// RETBR-OFF-NOT: "harden-sls-retbr"
+// RETBR-ON: "+harden-sls-retbr"
+
+// BLR-OFF-NOT: "harden-sls-blr"
+// BLR-ON: "+harden-sls-blr"
+
+// BAD-SLS-SPEC: invalid sls hardening option '{{[^']+}}' in '-mharden-sls=
+
+// RUN: %clang -target armv6a--none-eabi -c %s -### -mharden-sls=all 2>&1 | \
+// RUN: FileCheck %s --check-prefix=SLS-NOT-SUPPORTED
+
+// RUN: %clang -target armv6a--none-eabi -c %s -### -mharden-sls=retbr 2>&1 | \
+// RUN: FileCheck %s --check-prefix=SLS-NOT-SUPPORTED
+
+//
[PATCH] D93531: [clangd] Reuse buffer for JSONTransport::readRawMessage
njames93 updated this revision to Diff 312930.
njames93 added a comment.
Remove input buffers, but keep output as its easy to reason about.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93531/new/
https://reviews.llvm.org/D93531
Files:
clang-tools-extra/clangd/JSONTransport.cpp
Index: clang-tools-extra/clangd/JSONTransport.cpp
===
--- clang-tools-extra/clangd/JSONTransport.cpp
+++ clang-tools-extra/clangd/JSONTransport.cpp
@@ -126,13 +126,13 @@
bool handleMessage(llvm::json::Value Message, MessageHandler &Handler);
// Writes outgoing message to Out stream.
void sendMessage(llvm::json::Value Message) {
-std::string S;
-llvm::raw_string_ostream OS(S);
+OutputBuffer.clear();
+llvm::raw_svector_ostream OS(OutputBuffer);
OS << llvm::formatv(Pretty ? "{0:2}" : "{0}", Message);
-OS.flush();
-Out << "Content-Length: " << S.size() << "\r\n\r\n" << S;
+Out << "Content-Length: " << OutputBuffer.size() << "\r\n\r\n"
+<< OutputBuffer;
Out.flush();
-vlog(">>> {0}\n", S);
+vlog(">>> {0}\n", OutputBuffer);
}
// Read raw string messages from input stream.
@@ -143,6 +143,7 @@
llvm::Optional readDelimitedMessage();
llvm::Optional readStandardMessage();
+ llvm::SmallVector OutputBuffer;
std::FILE *In;
llvm::raw_ostream &Out;
llvm::raw_ostream &InMirror;
Index: clang-tools-extra/clangd/JSONTransport.cpp
===
--- clang-tools-extra/clangd/JSONTransport.cpp
+++ clang-tools-extra/clangd/JSONTransport.cpp
@@ -126,13 +126,13 @@
bool handleMessage(llvm::json::Value Message, MessageHandler &Handler);
// Writes outgoing message to Out stream.
void sendMessage(llvm::json::Value Message) {
-std::string S;
-llvm::raw_string_ostream OS(S);
+OutputBuffer.clear();
+llvm::raw_svector_ostream OS(OutputBuffer);
OS << llvm::formatv(Pretty ? "{0:2}" : "{0}", Message);
-OS.flush();
-Out << "Content-Length: " << S.size() << "\r\n\r\n" << S;
+Out << "Content-Length: " << OutputBuffer.size() << "\r\n\r\n"
+<< OutputBuffer;
Out.flush();
-vlog(">>> {0}\n", S);
+vlog(">>> {0}\n", OutputBuffer);
}
// Read raw string messages from input stream.
@@ -143,6 +143,7 @@
llvm::Optional readDelimitedMessage();
llvm::Optional readStandardMessage();
+ llvm::SmallVector OutputBuffer;
std::FILE *In;
llvm::raw_ostream &Out;
llvm::raw_ostream &InMirror;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D93528: [clang-format] Add basic support for formatting JSON
MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. > With my maintainer-of-`Support/JSON` hat on, I don't like the idea of these > features being shoehorned into the library to make clang-format an > incrementally more featureful JSON formatter. They're likely to lead to a lot > of conceptual+implementation+API complexity in a library that solves its > primary use cases quite well and simply. No plans at all to touch Support/JSON, if this needed a separate reformat() for json then I'd say I'm more likely to do that in lib/Format as we'd only need a "PrettyPrint" type function which supported the options we want to support > So if the plan is to > > - use this implementation to bootstrap JSON-in-clang-format That was kind of the plan > - with a firm intention of replacing it with parsing/formatting logic that > lives inside clang-format itself So I tried again to do this yesterday, but once again seems to fail, mainly around the area of clang-formatting trying to reflow and rejoin split lines but also the level of indentation is off. I'm starting to wonder if perhaps it needs its own separate pass, use more of clang-format but less of the actual "parsing into known structures"... > - long-term, maybe making use of `Support/JSON` long-term for some low level > bits (normalizing string tokens?) but not where flexibility is needed I think I'd really just like to use the `FormatTokens`, but I agree if we ended up writing a specific JSON pretty-printer I could see using the inner parts (but I really have no interesting in touching lib/Support/JSON at all) > then I think this is great. However I'm a bit leery if the plan is "land this > a MVP, and then play it by ear". Sorry I'm not sure I understand what MVP means in this context? Comment at: clang/unittests/Format/FormatTestJson.cpp:14 + +#define DEBUG_TYPE "format-test" + HazardyKnusperkeks wrote: > I don't know what's the practice right now. But I would suggest renaming that > to "format-test-json", so you can see directly that's for JSON. I'm not sure of the convention or even where/when its used (I just know you need it) lets take that change offline, we should either change them all or none of the them. ``` FormatTest.cpp:#define DEBUG_TYPE "format-test" FormatTestCSharp.cpp:#define DEBUG_TYPE "format-test" FormatTestComments.cpp:#define DEBUG_TYPE "format-test" FormatTestJS.cpp:#define DEBUG_TYPE "format-test" FormatTestJava.cpp:#define DEBUG_TYPE "format-test" FormatTestJson.cpp:#define DEBUG_TYPE "format-test" FormatTestObjC.cpp:#define DEBUG_TYPE "format-test" FormatTestProto.cpp:#define DEBUG_TYPE "format-test" FormatTestRawStrings.cpp:#define DEBUG_TYPE "format-test" FormatTestSelective.cpp:#define DEBUG_TYPE "format-test" FormatTestTableGen.cpp:#define DEBUG_TYPE "format-test" FormatTestTextProto.cpp:#define DEBUG_TYPE "format-test" NamespaceEndCommentsFixerTest.cpp:#define DEBUG_TYPE "namespace-end-comments-fixer-test" SortImportsTestJS.cpp:#define DEBUG_TYPE "format-test" SortImportsTestJava.cpp:#define DEBUG_TYPE "format-test" SortIncludesTest.cpp:#define DEBUG_TYPE "format-test" UsingDeclarationsSorterTest.cpp:#define DEBUG_TYPE "using-declarations-sorter-test" ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93528/new/ https://reviews.llvm.org/D93528 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D93446: [RISCV] Add vadd with mask and without mask builtin.
khchen added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:194 HasV = true; -else if (Feature == "+experimental-zfh") + HasRISCVVTypes = true; +} else if (Feature == "+experimental-zfh") HasRISCVVTypes is an undefined variable? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93446/new/ https://reviews.llvm.org/D93446 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D93540: [clang] Use enum for LangOptions::SYCLVersion instead of unsigned
bader accepted this revision. bader added subscribers: mdtoguchi, erichkeane. bader added a comment. This revision is now accepted and ready to land. LGTM. Thanks! @erichkeane, @mdtoguchi, we should upstream support for SYCL-2020 version in addition to SYCL-2017. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93540/new/ https://reviews.llvm.org/D93540 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87587: [clang-format][PR47290] Add MaxUnwrappedLinesForShortNamespace format option
kuzkry updated this revision to Diff 312940.
kuzkry retitled this revision from "[clang-format][PR47290] Make one-line
namespaces resistant to FixNamespaceComments, update documentation" to
"[clang-format][PR47290] Add MaxUnwrappedLinesForShortNamespace format option".
kuzkry edited the summary of this revision.
kuzkry added a comment.
As proposed by @MyDeveloperDay, I added MaxUnwrappedLinesForShortNamespace
option to control behaviour of FixNamespaceComments. This time I'm providing
you with a full diff.
Btw. sorry for taking so long.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87587/new/
https://reviews.llvm.org/D87587
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/NamespaceEndCommentsFixer.cpp
clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -816,7 +816,7 @@
"}\n"));
}
-TEST_F(NamespaceEndCommentsFixerTest, AddEndCommentForNamespacesAroundMacros) {
+TEST_F(NamespaceEndCommentsFixerTest, AddsEndCommentForNamespacesAroundMacros) {
// Conditional blocks around are fine
EXPECT_EQ("namespace A {\n"
"#if 1\n"
@@ -1116,6 +1116,92 @@
"}\n"
"}\n"));
}
+
+using MaxUnwrappedLinesForShortNamespaceTest = NamespaceEndCommentsFixerTest;
+
+TEST_F(MaxUnwrappedLinesForShortNamespaceTest, DefaultsToOneUnwrappedLine) {
+ constexpr auto UnwrappedLines = 1u;
+ auto const Style = getLLVMStyle();
+
+ EXPECT_EQ(UnwrappedLines, Style.MaxUnwrappedLinesForShortNamespace);
+}
+
+TEST_F(MaxUnwrappedLinesForShortNamespaceTest,
+ ZeroUnwrappedLines_DoesNotAddEndCommentForOneLinerNamespace) {
+ auto Style = getLLVMStyle();
+ Style.MaxUnwrappedLinesForShortNamespace = 0;
+
+ EXPECT_EQ("namespace OneLinerNamespace {}\n",
+fixNamespaceEndComments("namespace OneLinerNamespace {}\n", Style));
+}
+
+TEST_F(MaxUnwrappedLinesForShortNamespaceTest,
+ ZeroUnwrappedLines_DoesNotAddEndCommentForNonOneLinerNamespace) {
+ auto Style = getLLVMStyle();
+ Style.MaxUnwrappedLinesForShortNamespace = 0;
+
+ EXPECT_EQ("namespace NonOneLinerNamespace {\n"
+"}\n",
+fixNamespaceEndComments("namespace NonOneLinerNamespace {\n"
+"}\n",
+Style));
+}
+
+TEST_F(MaxUnwrappedLinesForShortNamespaceTest,
+ OneUnwrappedLine_DoesNotAddEndCommentForShortNamespace) {
+ EXPECT_EQ("namespace ShortNamespace {\n"
+"int i;\n"
+"}\n",
+fixNamespaceEndComments("namespace ShortNamespace {\n"
+"int i;\n"
+"}\n"));
+}
+
+TEST_F(MaxUnwrappedLinesForShortNamespaceTest,
+ OneUnwrappedLine_AddsEndCommentForLongNamespace) {
+ EXPECT_EQ("namespace LongNamespace {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace LongNamespace\n",
+fixNamespaceEndComments("namespace LongNamespace {\n"
+"int i;\n"
+"int j;\n"
+"}\n"));
+}
+
+TEST_F(MaxUnwrappedLinesForShortNamespaceTest,
+ MultipleUnwrappedLine_DoesNotAddEndCommentForShortNamespace) {
+ auto Style = getLLVMStyle();
+ Style.MaxUnwrappedLinesForShortNamespace = 2;
+
+ EXPECT_EQ("namespace ShortNamespace {\n"
+"int i;\n"
+"int j;\n"
+"}\n",
+fixNamespaceEndComments("namespace ShortNamespace {\n"
+"int i;\n"
+"int j;\n"
+"}\n",
+Style));
+}
+
+TEST_F(MaxUnwrappedLinesForShortNamespaceTest,
+ MultipleUnwrappedLine_AddsEndCommentForLongNamespace) {
+ auto Style = getLLVMStyle();
+ Style.MaxUnwrappedLinesForShortNamespace = 2;
+
+ EXPECT_EQ("namespace LongNamespace {\n"
+"int i;\n"
+"int j;\n"
+"int k;\n"
+"}// namespace LongNamespace\n",
+fixNamespaceEndComments("namespace LongNamespace {\n"
+"int i;\n"
+"int j;\n"
+"int k;\n"
+"}\n",
+Style));
+}
} // end namespace
} // end namespace format
} // end namespace clang
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndC
[PATCH] D87587: [clang-format][PR47290] Add MaxUnwrappedLinesForShortNamespace format option
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. Thanks for making the changes, this LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87587/new/ https://reviews.llvm.org/D87587 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.
zoecarver added a comment. > We could probably do something like what this patch is doing and determine > whether a class can be passed in registers based on whether its subobjects > can be passed in registers. If all of the subobjects can be passed in > registers, the current class can be passed in registers too unless something > declared in the current class forces it to be passed indirectly (e.g., a > virtual function is declared). That's where I'm getting confused because on main both `B0` and `B1` are passed directly, so why isn't `D` also getting passed directly? There's nothing declared in that class other than the two members. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92361/new/ https://reviews.llvm.org/D92361 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D93587: [hip] Fix HIP version parsing.
hliao created this revision.
hliao added reviewers: yaxunl, tra.
Herald added subscribers: kerbowa, nhaehnle, jvesely.
hliao requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
- Need trimming before parsing major or minor version numbers. This's required
due to the different line ending on Windows.
- In addition, the integer conversion may fail due to invalid char. Return that
parsing function return `true` when the parsing fails.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D93587
Files:
clang/lib/Driver/ToolChains/AMDGPU.cpp
clang/lib/Driver/ToolChains/ROCm.h
clang/test/Driver/Inputs/rocm/bin/.hipVersion
Index: clang/test/Driver/Inputs/rocm/bin/.hipVersion
===
--- clang/test/Driver/Inputs/rocm/bin/.hipVersion
+++ clang/test/Driver/Inputs/rocm/bin/.hipVersion
@@ -1,4 +1,6 @@
# Auto-generated by cmake
-HIP_VERSION_MAJOR=3
+# NOTE: The trailing whitespace is added on purpose to verify that these
+# whitespaces are trimmed before paring.
+HIP_VERSION_MAJOR=3
HIP_VERSION_MINOR=6
HIP_VERSION_PATCH=20214-a2917cd
Index: clang/lib/Driver/ToolChains/ROCm.h
===
--- clang/lib/Driver/ToolChains/ROCm.h
+++ clang/lib/Driver/ToolChains/ROCm.h
@@ -103,7 +103,7 @@
}
void scanLibDevicePath(llvm::StringRef Path);
- void ParseHIPVersionFile(llvm::StringRef V);
+ bool parseHIPVersionFile(llvm::StringRef V);
SmallVector getInstallationPathCandidates();
public:
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -88,23 +88,28 @@
}
}
-void RocmInstallationDetector::ParseHIPVersionFile(llvm::StringRef V) {
+bool RocmInstallationDetector::parseHIPVersionFile(llvm::StringRef V) {
SmallVector VersionParts;
V.split(VersionParts, '\n');
- unsigned Major;
- unsigned Minor;
+ unsigned Major = ~0U;
+ unsigned Minor = ~0U;
for (auto Part : VersionParts) {
auto Splits = Part.split('=');
-if (Splits.first == "HIP_VERSION_MAJOR")
- Splits.second.getAsInteger(0, Major);
-else if (Splits.first == "HIP_VERSION_MINOR")
- Splits.second.getAsInteger(0, Minor);
-else if (Splits.first == "HIP_VERSION_PATCH")
- VersionPatch = Splits.second.str();
+if (Splits.first == "HIP_VERSION_MAJOR") {
+ if (Splits.second.trim().getAsInteger(0, Major))
+return true;
+} else if (Splits.first == "HIP_VERSION_MINOR") {
+ if (Splits.second.trim().getAsInteger(0, Minor))
+return true;
+} else if (Splits.first == "HIP_VERSION_PATCH")
+ VersionPatch = Splits.second.trim().str();
}
+ if (Major == ~0U || Minor == ~0U)
+return true;
VersionMajorMinor = llvm::VersionTuple(Major, Minor);
DetectedVersion =
(Twine(Major) + "." + Twine(Minor) + "." + VersionPatch).str();
+ return false;
}
// For candidate specified by --rocm-path we do not do strict check.
@@ -290,7 +295,8 @@
continue;
if (HIPVersionArg.empty() && VersionFile)
- ParseHIPVersionFile((*VersionFile)->getBuffer());
+ if (parseHIPVersionFile((*VersionFile)->getBuffer()))
+continue;
HasHIPRuntime = true;
return;
Index: clang/test/Driver/Inputs/rocm/bin/.hipVersion
===
--- clang/test/Driver/Inputs/rocm/bin/.hipVersion
+++ clang/test/Driver/Inputs/rocm/bin/.hipVersion
@@ -1,4 +1,6 @@
# Auto-generated by cmake
-HIP_VERSION_MAJOR=3
+# NOTE: The trailing whitespace is added on purpose to verify that these
+# whitespaces are trimmed before paring.
+HIP_VERSION_MAJOR=3
HIP_VERSION_MINOR=6
HIP_VERSION_PATCH=20214-a2917cd
Index: clang/lib/Driver/ToolChains/ROCm.h
===
--- clang/lib/Driver/ToolChains/ROCm.h
+++ clang/lib/Driver/ToolChains/ROCm.h
@@ -103,7 +103,7 @@
}
void scanLibDevicePath(llvm::StringRef Path);
- void ParseHIPVersionFile(llvm::StringRef V);
+ bool parseHIPVersionFile(llvm::StringRef V);
SmallVector getInstallationPathCandidates();
public:
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -88,23 +88,28 @@
}
}
-void RocmInstallationDetector::ParseHIPVersionFile(llvm::StringRef V) {
+bool RocmInstallationDetector::parseHIPVersionFile(llvm::StringRef V) {
SmallVector VersionParts;
V.split(VersionParts, '\n');
- unsigned Major;
- unsigned Minor;
+ unsigned Major = ~0U;
+ unsigned Minor = ~0U;
for (auto Part : VersionParts) {
auto Splits = Part.split('=');
-if (Splits.first == "HIP_VERSION_MAJOR")
- Splits.second.getAsInteger(0, Major);
-else
[PATCH] D93446: [RISCV] Add vadd with mask and without mask builtin.
HsiangKai added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:194 HasV = true; -else if (Feature == "+experimental-zfh") + HasRISCVVTypes = true; +} else if (Feature == "+experimental-zfh") khchen wrote: > HasRISCVVTypes is an undefined variable? I should move the snippet to D92715. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93446/new/ https://reviews.llvm.org/D93446 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D93452: [clangd] Trim memory periodically
njames93 added a comment. Just attached a debugger and ran malloc_trim on my clangd instance which was sitting at 6.7GB, afterwards it was 1.3GB. Interestingly clangd held rock solid at 1.0GB recorded memory usage throughout. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93452/new/ https://reviews.llvm.org/D93452 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128
hubert.reinterpretcast added inline comments.
Comment at: clang/test/CodeGenCXX/ibm128-declarations.cpp:25
+
+__ibm128 func4(__ibm128 a, __ibm128 b) {
+ return a + b;
hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > There's a lot of missing testing, especially around implicit conversions,
> > narrowing conversions, and the usual arithmetic conversions.
> >
> Make this a `constexpr` function and call it from the initializer of a global
> declared with `consteval`.
> Make this a `constexpr` function and call it from the initializer of a global
> declared with `consteval`.
Sorry, I meant `constinit` and not `consteval`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93377/new/
https://reviews.llvm.org/D93377
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
