[PATCH] D154869: [Flang] [FlangRT] Introduce FlangRT project as solution to Flang's runtime LLVM integration

DavidSpickett added a comment. I've reverted this https://github.com/llvm/llvm-project/commit/ffc67bb3602a6a9a4f886af362e1f2d7c9821570 as Linaro's flang and clang bots are red all over and we're also in an upgrade period for the build server (https://discourse.llvm.org/t/llvm-zorg-migration-to

[PATCH] D49549: Change 'clang-test' to 'check-clang' on the hacking webpage

DavidSpickett closed this revision. DavidSpickett added a comment. Herald added a project: All. I updated this to the monorepo layout and landed it: https://github.com/llvm/llvm-project/commit/604a231881a0e8f204a6d3337bfa3e797e911e34 Given the long time this has been here and the last activity o

[PATCH] D119008: Add Cortex-X1C to Clang LLVM 14 release notes

DavidSpickett closed this revision. DavidSpickett added a comment. Herald added a project: All. Landed on 14.x as https://github.com/llvm/llvm-project/commit/56dcb10a9942c206d53a59abd66c2207a01b39fa. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D11

[PATCH] D74031: Update for Clang 10 release notes in order to have reference to D66404.

DavidSpickett closed this revision. DavidSpickett added a comment. Herald added a project: All. Was landed on 10.x as https://github.com/llvm/llvm-project/commit/b4efc29f1ccbc03453590bf7aae337853c91c91f. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D154093: [clang-format] Break long string literals in C#, etc.

DavidSpickett added a comment. This was causing failures on a large number of Linaro's bots. Judging by how widespread it was, reproducing could be as simple as adding `-DLLVM_ENABLE_ASSERTIONS=True` to your CMake config. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D158307: [flang][driver] Disable Clang options in Flang

DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. For reasons I won't get into, buildbots don't show what llvm-test-suite revision they are using but we can be sure by now that the other change has gone through. Fingers crossed

[PATCH] D158289: [flang][driver] Partial revert of https://reviews.llvm.org/D157837

DavidSpickett accepted this revision. DavidSpickett added a comment. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158289/new/ https://reviews.llvm.org/D158289 ___ cfe-commits mailing list cfe-comm

[PATCH] D156571: [DebugInfo] Alternate MD5 fix, NFC

DavidSpickett added a comment. Failing on one of our 2 stage AArch64 bots https://lab.llvm.org/buildbot/#/builders/176/builds/4267 after the reland (that bot has been red for various reasons today, so no emails got sent). Maybe it helps to know that it doesn't fail on the same config with singl

[PATCH] D158218: [CMake] Deprecate DEFAULT_SYSROOT and GCC_INSTALL_PREFIX

DavidSpickett resigned from this revision. DavidSpickett added a comment. I do cross build lldb a lot but never needed to set either of these, so I have no opinion on this. > clang/cmake/caches/CrossWinToARMLinux.cmake Is not one of Linaro's, there's a buildbot for it (https://lab.llvm.org/bui

[PATCH] D157808: [clang] Add missing field to VisibilityAttr json AST dump

DavidSpickett added a comment. We also had a failure looking for the mangled name on Windows on Arm: https://lab.llvm.org/buildbot/#/builders/65/builds/10954 The triple fix may take care of that too. > Also, consider spreading out commits a bit so that if one breaks something, > it's easily to

[PATCH] D76096: [clang] allow const structs/unions/arrays to be constant expressions for C

DavidSpickett added a comment. One of the kernel tests now fails to build for AArch64 and Arm: 00:00:48 ++ make CC=/home/tcwg-buildslave/workspace/tcwg_kernel_0/bin/aarch64-cc LD=/home/tcwg-buildslave/workspace/tcwg_kernel_0/llvm-install/bin/ld.lld SUBLEVEL=0 EXTRAVERSION=-bisect ARCH=arm64

[PATCH] D146418: Support for OpenMP 5.0 sec 2.12.7 - Declare Target initializer expressions

DavidSpickett added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:23148 + GlobalDeclRefChecker Checker; + if (auto *TargetVarDecl = dyn_cast_or_null(TargetDecl)) +Checker.declareTargetInitializer(TargetDecl); FYI I fixed an unused var warning h

[PATCH] D153430: [Clang][Driver] Warn on invalid Arm or AArch64 baremetal target triple

DavidSpickett accepted this revision. DavidSpickett added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153430/new/ https://reviews.llvm.org/D153430 ___ cfe-commits mailing list cfe-commi

[PATCH] D153430: [Clang][Driver] Warn on invalid Arm or AArch64 baremetal target triple

DavidSpickett added a comment. > Does that make it clearer? (I still suspect that people won't understand what > is meant by "environment" here but I can't think of a better explanation). "clang triple environment" into Google gets you the cross compilation guide and the Triple class docs so "e

[PATCH] D153430: Warn on invalid Arm or AArch64 baremetal target triple

DavidSpickett added a comment. > Perhaps "Unrecognized environment for , did you mean > -none-" I like explaining it like this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153430/new/ https://reviews.llvm.org/D153430 _

[PATCH] D153430: Warn on invalid Arm or AArch64 baremetal target triple

DavidSpickett added a comment. Please tag the commit title with the subproject, `[clang]` in this case. If only to help your friendly neighborhood buildbot minder. I'd maybe go with "Did you mean" instead of "try" but only because the first thing I think is well if I try it what will I get. The

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

DavidSpickett added inline comments. Comment at: clang/test/CodeGen/RISCV/riscv-inline-asm-gcc-commenting.c:23 +// CHECK: f1: +// CHECK: lui a0, 1 +// CHECK-NEXT: addiw a0, a0, 564 Check for the comment content here too? `# this is fine # this

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

DavidSpickett added a reviewer: DavidSpickett. DavidSpickett added a comment. Is the comment here relevant? https://discourse.llvm.org/t/interleaving-several-c-style-comments-in-the-same-inline-assembly-line/71353/8 Does this patch do that already? Also is it a problem that the ignored comments

[PATCH] D152070: [2/11][Clang][RISCV] Expand all variants of RVV intrinsic tuple types

DavidSpickett added a comment. FYI after this change: Building CXX object tools/lldb/sou...luginTypeSystemClang.dir/TypeSystemClang.cpp.o /home/david.spickett/llvm-project/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:4843:13: warning: 225 enumeration values not handled in switc

[PATCH] D148783: [clangd] Add support TextDocumentEdit.

DavidSpickett added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148783/new/ https://reviews.llvm.org/D148783 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D148783: [clangd] Add support TextDocumentEdit.

DavidSpickett added a comment. Hi, these tests are failing on our (Linaro's) Windows on Arm buildbot. First appearance here https://lab.llvm.org/buildbot/#/builders/65/builds/9346 Still failing as of a few hours ago https://lab.llvm.org/buildbot/#/builders/65/builds/9360 (maybe you have fixed

[PATCH] D148799: [clang] Return `std::string_view` from `TargetInfo::getClobbers()`

DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. If you have commit access already, wait until the pre-commit builds finish and check there isn't anything failing there. If you don't, I can commit this for you as I did before.

[PATCH] D148799: [clang] Return `std::string_view` from `TargetInfo::getClobbers()`

DavidSpickett added a comment. Build issues will also show up in the "Build Status" here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148799/new/ https://reviews.llvm.org/D148799 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D148799: [clang] Return `std::string_view` from `TargetInfo::getClobbers()`

DavidSpickett added a comment. > I did not set const qualifier for return type because std::string_view is > constant by design. Or should I mark it const explicitly? I'm new to string_view but everything I see backs up that it is a constant view on the data as you say. I'm not sure what making

[PATCH] D148799: [clang] Return `StringRef` from `TargetInfo::getClobbers()`

DavidSpickett added a comment. string_view is a good call, especially as this is not doing anything fancy with it, and we have recently been allowed to use it. > (Maybe) Make private static field of std::string type for actual clobber > value (or std::string_view type since it can be constexpr-

[PATCH] D148799: [clang] Return `StringRef` from `TargetInfo::getClobbers()`

DavidSpickett added a comment. You need to clearly state the "why" of the change in the commit message. Sometimes it seems obvious to the author but a reviewer has to assume and may assume incorrectly. In this case, I guess that since the result of the function only has its length checked then

[PATCH] D148799: [clang] Return `StringRef` from `TargetInfo::getClobbers()`

DavidSpickett added a comment. > Context not available. Please update the diff with context, https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface. I'm not sure what the benefit of this change is. While I did say less raw pointers = better, perhaps I'd make an except

[PATCH] D148574: [clang] Return std::unique_ptr from AllocateTarget

DavidSpickett added a comment. > What should I do further? And contribute more if you like :) Thanks for the contribution! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148574/new/ https://reviews.llvm.org/D148574 ___

[PATCH] D148574: [clang] Return std::unique_ptr from AllocateTarget

DavidSpickett added a comment. You may get some emails about build failures. Try to judge if they are relevant to you, ask here if you are not sure. I doubt it will do much given that it's just refactoring and both uses seem to be GPU offload targets. Repository: rG LLVM Github Monorepo CH

[PATCH] D148574: [clang] Return std::unique_ptr from AllocateTarget

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG830b359d3ab1: [clang] Return std::unique_ptr from AllocateTarget (authored by Stoorx, committed by DavidSpickett). Repository: rG LLV

[PATCH] D148574: [clang] Return std::unique_ptr from AllocateTarget

DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. > Or should I provide a real name? Username is fine as long as you're fine being credited as such. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D148574: [clang] Use the 'std::unique_ptr' instead of a raw pointers

DavidSpickett added a comment. I'd suggest rewording the title a bit: `[clang] Return std::unique_ptr from AllocateTarget` Titles are always weird but at least including the function name means someone staring at a build failure can skim for likely commits. It's not always possible though. Sa

[PATCH] D148574: [clang] Use the 'std::unique_ptr' instead of a raw pointers

DavidSpickett added a comment. > That's my first commit into LLVM project. (Almost just for test of the > contribution procedure.) Welcome! > Maybe I did something wrong, but the build fails on example tests. Do the tests fail locally? I will look at the pre-commit logs as well. It may be fai

[PATCH] D148529: [clang] Replace find_executable with shutil.which in creduce script

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGee3413736251: [clang] Replace find_executable with shutil.which in creduce script (authored by DavidSpickett). Repository: rG LLVM Github Monorepo

[PATCH] D148529: [clang] Replace find_executable with shutil.which in creduce script

DavidSpickett created this revision. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. distutils is deprecated and shutil.which is the suggested replacement for this function. https://peps.python.or

cfe-commits@lists.llvm.org

DavidSpickett added a comment. I've reverted this, please try to fix the test then reland. The full test output can be downloaded from the buildbot page, if you need any more information let me know. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

cfe-commits@lists.llvm.org

DavidSpickett added a comment. The test is also failing on our Windows on Arm bot: https://lab.llvm.org/buildbot/#/builders/65/builds/8950 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143128/new/ https://reviews.llvm.org/D143128

[PATCH] D143704: [flang] Feature list plugin

DavidSpickett added a comment. Specifically https://lab.llvm.org/buildbot/#/builders/181/builds/15552. Though it is only the sharedlibs build failing, I don't know why. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143704/new/ https://reviews.llvm

[PATCH] D146041: Fix weirdly apologetic diagnostic messages

DavidSpickett added a comment. > Since this is my first commit to such a large repository(and project), can > you please guide me with this @DavidSpickett ! Sure, you'll want to make a commit that only has changes to warnings and errors. You can split up this one to do that, see part "A)" of th

[PATCH] D146041: Fix weirdly apologetic diagnostic messages

DavidSpickett added a comment. Ah, here it is: https://llvm.org/docs/CodingStandards.html#error-and-warning-messages Seems to back you up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146041/new/ https://reviews.llvm.org/D146041 ___

[PATCH] D146041: Fix weirdly apologetic diagnostic messages

DavidSpickett added a comment. Is there some standard for writing warning messages? For llvm that is, it would be worth looking through the getting started guides to see. I think the majority of warnings are "formal" in that sense so this seems fine. Personally I agree with making the warnings

[PATCH] D145227: [LLVM][OHOS] Clang toolchain and targets

DavidSpickett added a comment. Also failing on our Windows on Arm bot: https://lab.llvm.org/buildbot/#/builders/65/builds/8607 I think some lines need to account for Windows slashes, `{{/|}}` should do that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D145227: [LLVM][OHOS] Clang toolchain and targets

DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. Looks good in general. If there are some incorrect details you'll find them later. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D145781: [AArch64] Don't #define __ARM_FP when there's no FPU.

DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145781/new/ https://reviews.llvm.org/D145781

[PATCH] D145227: [LLVM][OHOS] Clang toolchain and targets

DavidSpickett added a comment. I'm not familiar with the details of toolchain config, but added some general comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1404 + // OHOS-specific defaults for PIC/PIE + if (Triple.isOHOSFamily()) { +switch (Triple.getAr

[PATCH] D135495: [clang-tidy] handle exceptions properly in `ExceptionAnalyzer`

DavidSpickett added a comment. If it helps, the bot I linked is pretty vanilla. You can find all the details in the buildbot UI but just in case: cmake -G Ninja ../llvm/llvm -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=True '-DLLVM_LIT_ARGS='"'"'-v'"'"'' -DCMAKE_INSTALL_PREFIX=../stag

[PATCH] D135495: [clang-tidy] handle exceptions properly in `ExceptionAnalyzer`

DavidSpickett added a comment. The test was timing out on Arm/AArch64 bots so I have reverted this change. I think because there were other test failures on the initial run, that obscured the time out. This is one of the first builds to include this change: https://lab.llvm.org/buildbot/#/build

[PATCH] D143052: [CMake] Replace llvm_check_linker_flag and llvm_check_compiler_linker_flag

DavidSpickett accepted this revision. DavidSpickett added a comment. https://cmake.org/cmake/help/v3.18/module/CheckLinkerFlag.html Looks equivalent to me, good timing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143052/new/ https://reviews.llvm

[PATCH] D142822: [clang] ASTImporter: Fix importing of va_list types and declarations

DavidSpickett added a comment. On AArch64 we have the following failures: Failed Tests (15): Clang :: CXX/basic/basic.stc/basic.stc.dynamic/p2-nodef.cpp Clang :: CodeCompletion/ordinary-name-cxx11.cpp Clang :: CodeCompletion/ordinary-name.cpp Clang :: CodeGenCXX/cxx20-module-std

[PATCH] D140011: [clang][compiler-rt] Support LLVM_ENABLE_PER_TARGET_RUNTIME_DIR on Arm Linux and BSD

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcd173cbd7cca: [clang][compiler-rt] Support LLVM_ENABLE_PER_TARGET_RUNTIME_DIR on Arm Linux… (authored by DavidSpickett). Repository: rG LLVM Githu

[PATCH] D140011: [clang][compiler-rt] Support LLVM_ENABLE_PER_TARGET_RUNTIME_DIR on Arm Linux and BSD

DavidSpickett updated this revision to Diff 493518. DavidSpickett added a comment. Remove the Armhf comment, rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140011/new/ https://reviews.llvm.org/D140011 Files: clang/lib/Driver/ToolChain.cpp

[PATCH] D141796: [15/15][Clang][RISCV][NFC] Set data member under Policy as constants

DavidSpickett added a comment. Looks good to me - https://lab.llvm.org/buildbot/#/builders/245/builds/3919 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141796/new/ https://reviews.llvm.org/D141796 ___ c

[PATCH] D141796: [15/15][Clang][RISCV][NFC] Set data member under Policy as constants

DavidSpickett added a comment. Nice! Glad you spotted that, I have had creduce running and getting nowhere fast for a day or so. Sounds like it would never have found anything minimal. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141796/new/ htt

[PATCH] D136886: [clang] ASTImporter: Fix importing of va_list types and declarations

DavidSpickett added a comment. I am attempting to reduce the compiler crash on 32 bit, it's a slow process. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136886/new/ https://reviews.llvm.org/D136886 ___

[PATCH] D141796: [15/15][Clang][RISCV][NFC] Set data member under Policy as constants

DavidSpickett added a comment. Hi! Comparing the changes for https://lab.llvm.org/buildbot/#/builders/17/builds/33129 and https://lab.llvm.org/buildbot/#/builders/174/builds/17069 I think you've managed to hit a crash in the host compiler we (Linaro) are using for our bots. Which is the 15.0.0

[PATCH] D136886: [clang] ASTImporter: Fix importing of va_list types and declarations

DavidSpickett added a comment. I was able to reproduce on AArch64 and Arm (32 bit), did not reproduce on an Intel machine which matches what you've seen so far. 62 failures on AArch64, 21 on Arm. Broadly two kinds of failure. Errors of the form `error: reference to 'std' is ambiguous`, which ha

[PATCH] D136886: [clang] ASTImporter: Fix importing of va_list types and declarations

DavidSpickett added a comment. Hi folks, I'm one of the owners of the AArch64 bot you're seeing the failure on. I will try to reproduce it myself and see if I can find what's going on. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136886/new/ http

[PATCH] D140011: [clang][compiler-rt] Support LLVM_ENABLE_PER_TARGET_RUNTIME_DIR on Arm Linux and BSD

DavidSpickett added a comment. I will wait until after the 16 branch happens to land this. The release build already has some issues and I won't have time to do the fixups in the next few weeks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140011

[PATCH] D140011: [clang][compiler-rt] Support LLVM_ENABLE_PER_TARGET_RUNTIME_DIR on Arm Linux and BSD

DavidSpickett updated this revision to Diff 483112. DavidSpickett added a comment. Expand the comment and show an example of changed triples. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140011/new/ https://reviews.llvm.org/D140011 Files: clang

[PATCH] D140011: [clang][compiler-rt] Support LLVM_ENABLE_PER_TARGET_RUNTIME_DIR on Arm Linux and BSD

DavidSpickett added a comment. I haven't tested this on a BSD myself. I don't see a major reason it wouldn't work but if you think it's going to have issues I can do that testing and/or restrict this change to Linux only initially. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D140011: [clang][compiler-rt] Support LLVM_ENABLE_PER_TARGET_RUNTIME_DIR on Arm Linux and BSD

DavidSpickett updated this revision to Diff 482811. DavidSpickett added a comment. - `-target` changed to `--target=` in test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140011/new/ https://reviews.llvm.org/D140011 Files: clang/lib/Driver/Too

[PATCH] D140011: [clang][compiler-rt] Support LLVM_ENABLE_PER_TARGET_RUNTIME_DIR on Arm Linux and BSD

DavidSpickett added reviewers: MaskRay, phosek. DavidSpickett added a comment. Herald added a subscriber: StephenFan. The first attempt at this was https://reviews.llvm.org/D110142. The key difference is that... > It also seems like armhf-unknown-linux-gnueabihf duplicates the hf bit > (there's

[PATCH] D140011: [clang][compiler-rt] Support LLVM_ENABLE_PER_TARGET_RUNTIME_DIR on Arm Linux and BSD

DavidSpickett created this revision. Herald added subscribers: Enna1, kristof.beyls, dberris. Herald added a project: All. DavidSpickett requested review of this revision. Herald added subscribers: llvm-commits, Sanitizers, cfe-commits, MaskRay. Herald added projects: clang, Sanitizers, LLVM. The

[PATCH] D137836: [Support] Move getHostNumPhysicalCores to Threading.h

DavidSpickett added a comment. Looks like tests need updating for that new -1 return value: https://lab.llvm.org/buildbot/#/builders/178/builds/3419 That bot builds with threading disabled. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137836/new/

[PATCH] D138792: [AArch64] Improve TargetParser API

DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. Looks like a good direction to me. Plenty more bits in clang that should really live in the target parser, and this is a great start at moving those. My only issue is the use of

[PATCH] D138157: Make -fsanitize=scudo use scudo_standalone. Delete check-scudo.

DavidSpickett added a comment. We (Linaro) also have an issue with a bot that uses `-DCOMPILER_RT_BUILD_SANITIZERS=OFF`. https://lab.llvm.org/buildbot/#/builders/178/builds/3318 CMake Error at cmake/modules/AddLLVM.cmake:1915 (add_dependencies): The dependency target "ScudoUnitTests" of t

[PATCH] D138539: Use std::nullopt_t instead of NoneType (NFC)

DavidSpickett added a reviewer: DavidSpickett. DavidSpickett added a comment. lldb parts LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138539/new/ https://reviews.llvm.org/D138539 ___ cfe-commits m

[PATCH] D137338: Fix dupe word typos

DavidSpickett added a comment. Looked at the lldb changes, some comments for you. If you want to get a "looks good" for those please submit a separate review with only the lldb parts and I'll review that instead. As others said, appreciate the effort but the review process doesn't scale well t

[PATCH] D135583: [LLVM] Use DWARFv4 bitfields when tuning for GDB

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc6e2de60423a: [LLVM] Use DWARFv4 bitfields when tuning for GDB (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D135583: [LLVM] Use DWARFv4 bitfields when tuning for GDB

DavidSpickett updated this revision to Diff 471065. DavidSpickett added a comment. - DWARFvN -> DWARF vN - Remove superfluous brackets. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135583/new/ https://reviews.llvm.org/D135583 Files: clang/docs/

[PATCH] D135583: [LLVM] Use DWARFv4 bitfields when tuning for GDB

DavidSpickett updated this revision to Diff 470766. DavidSpickett added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. - Test that linux DWARFv4 uses data_bit_offset and that adding GDB tuning does not change that. - Add release notes for clang and llvm. Repos

[PATCH] D136124: [clang][deps] Remove unintentional `move`

DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. LGTM As it happens the bot that found this has been moved to silent for other reasons. I'll let you know if we see any further issues. Repository: rG LLVM Github Monorepo CH

[PATCH] D136124: [clang][deps] Remove unintentional `move`

DavidSpickett added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:400 +auto OverlayFS = +llvm::makeIntrusiveRefCnt(BaseFS); auto InMemoryFS = Is this equivalent? ``` auto OverlayFS = BaseFS; ``` G

[PATCH] D135414: [clang][deps] Don't share in-memory VFS across scans

DavidSpickett added a comment. Hi @jansvoboda11 , we (Linaro) happen to be one of the few running a bot with threading disabled (for reasons that aren't important here). It has been red a lot lately so you won't have seen the report from it. https://lab.llvm.org/buildbot/#/builders/178/builds/3

[PATCH] D136070: [LIT] Add AArch64/Windows to LP64 feature

DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136070/new/ https://reviews.llvm.org/D136070 _

[PATCH] D135461: [LLDB] Fix crash when printing a struct with a static wchar_t member

DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. > although those changes don't actually test the code path changed here Yeah I'm just cargo culting on that one so it doesn't look strange that a few are missing. If we're going

[PATCH] D135461: [LLDB] Fix crash when printing a struct with a static wchar_t member

DavidSpickett added a comment. You need to update the python file for the test as well. What does the output look like, do we actually print the character itself or it's value? (just curious, which one we do is probably out of scope for this change) Comment at: clang/lib/AST

[PATCH] D135170: [LLDB] Fix crash when printing a struct with a static signed char member

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5a9e21305803: [LLDB] Fix crash when printing a struct with a static signed char member (authored by DavidSpickett). Repository: rG LLVM Github Mon

[PATCH] D135169: [LLDB] Fix printing a static bool struct member when using "image lookup -t"

This revision was automatically updated to reflect the committed changes. Closed by commit rG02c1c939486f: [LLDB] Fix printing a static bool struct member when using "image lookup -t" (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D134788: [ARM64EC][clang-cl] Add arm64EC test; NFC

DavidSpickett added a comment. This (and it's followup?) has been landed, right? Please close the revision if so. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134788/new/ https://reviews.llvm.org/D134788 ___ cfe-commits mailing list cfe-com

[PATCH] D135256: [clang] Add Create method for CXXBoolLiteralExpr

This revision was automatically updated to reflect the committed changes. Closed by commit rG7448f38898a8: [clang] Add Create method for CXXBoolLiteralExpr (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135256/new/ https:

[PATCH] D135170: [LLDB] Fix crash when printing a struct with a static signed char member

DavidSpickett added inline comments. Comment at: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp:39 const static auto char_min = std::numeric_limits::min(); const static auto uchar_min = std::numeric_limits::min(); shafik wrote: > We use `sig

[PATCH] D135170: [LLDB] Fix crash when printing a struct with a static signed char member

DavidSpickett updated this revision to Diff 465342. DavidSpickett added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135170/new/ https://reviews.llvm.org/D135170 Files: clang/lib/AST/StmtPrinter.cpp lldb/test/API/lang/cpp/c

[PATCH] D135169: [LLDB] Fix printing a static bool struct member when using "image lookup -t"

DavidSpickett updated this revision to Diff 465341. DavidSpickett added a comment. Rebase, Create moved to parent change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135169/new/ https://reviews.llvm.org/D135169 Files: lldb/include/lldb/Symbol/

[PATCH] D135256: [clang] Add Create method for CXXBoolLiteralExpr

DavidSpickett created this revision. Herald added a subscriber: martong. Herald added a reviewer: shafik. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo http

[PATCH] D135169: [LLDB] Fix printing a static bool struct member when using "image lookup -t"

DavidSpickett added inline comments. Comment at: clang/include/clang/AST/ExprCXX.h:733 + static CXXBoolLiteralExpr *Create(const ASTContext &C, bool Val, QualType Ty, +SourceLocation Loc) { shafik wrote: > I think this makes

[PATCH] D135169: [LLDB] Fix printing a static bool struct member when using "image lookup -t"

DavidSpickett added a comment. Herald added a subscriber: JDevlieghere. For https://github.com/llvm/llvm-project/issues/58135. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135169/new/ https://reviews.llvm.org/D135169 _

[PATCH] D135170: [LLDB] Fix crash when printing a struct with a static signed char member

DavidSpickett created this revision. Herald added a project: All. DavidSpickett requested review of this revision. Herald added projects: clang, LLDB. Herald added subscribers: lldb-commits, cfe-commits. As with static bool for whatever reason printing them on their own worked fine but wasn't hand

[PATCH] D135169: [LLDB] Fix printing a static bool struct member when using "image lookup -t"

DavidSpickett created this revision. Herald added a reviewer: shafik. Herald added a project: All. DavidSpickett requested review of this revision. Herald added projects: clang, LLDB. Herald added subscribers: lldb-commits, cfe-commits. Fixes #58135 Somehow lldb was able to print the member on it

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

DavidSpickett added a comment. This looks fine as is, all comments addressed as far as I can see. Comment at: clang/lib/Driver/Driver.cpp:1384 + TC.getTriple().getSubArch() != llvm::Triple::AArch64SubArch_arm64ec) { +if (UArgs->hasArg(options::OPT__SLASH_arm64EC)) { +

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. Please mark comments as done if you feel that they have been addressed. (I know this can be a bit weird, do you mark them or do I, I go with the former, I can disagree if needed)

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

DavidSpickett added a comment. Has Microsoft documented this option? Not doubting it exists but I mostly see how to use it rather than a specific page describing the option and its behaviour. Not a big deal but if there is one, please include a link to it in the commit message.

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

DavidSpickett added a comment. +1 from me also, but someone else should check that this is a reasonable way to implement it cmake wise (not that this is a horrible hack but I never can tell with cmake). One more question, does this same issue potentially apply to llvm-tblgen and has that got a

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

DavidSpickett added inline comments. Comment at: clang/lib/Support/CMakeLists.txt:21 + # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't + # accidentally link against libLLVMSupport twice (once statically and once via + # libLLVM-*.so). -

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

DavidSpickett added a comment. GCC also rejects those tests so https://reviews.llvm.org/D134417 to disable them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133711/new/ https://reviews.llvm.org/D133711 __

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

DavidSpickett added a comment. This has caused 2 test suite failures across our bots, for example: https://lab.llvm.org/buildbot/#/builders/174/builds/13518 `GCC-C-execute-pr36093.test` / `GCC-C-execute-pr43783.test` Pretty likely the tests are doing something strange, I will look into it. Re

[PATCH] D125419: [Arm64EC 7/?] clang side of Arm64EC varargs ABI.

DavidSpickett added a comment. I don't have the expertise to approve, but one question. Why is there more testing in the `.c` than in `.cpp`. Is the same logic being used for both so there's no point checking twice? If so what things is the `.cpp` test looking for specifically? I struggle to s

[PATCH] D133109: [LLVM][ARM] Remove options for armv2, 2A, 3 and 3M

DavidSpickett added subscribers: nickdesaulniers, psmith. DavidSpickett added a comment. > 1:38 AM might be good to take them out here as well So I looked into this. Here are the Arm architectures that clang has that gcc doesn't: "armv5tej" // Not in GCC, j = jazelle "armv7k" // Apple Watch S1 "

[PATCH] D113779: [Clang] Add mfp16, mfp16fml and mdotprod flags for ARM target features.

DavidSpickett added a comment. For (1) and (2) is there a need to be able to reset the architecture setting no matter the previous `march` and `` are? Currently we're talking about *not* wanting having to use `-march=armv8-a+crc` but would you still want a way to reset the architecture no matte

[PATCH] D133109: [LLVM][ARM] Remove options for armv2, 2A, 3 and 3M

This revision was automatically updated to reflect the committed changes. Closed by commit rGe428baf0019e: [LLVM][ARM] Remove options for armv2, 2A, 3 and 3M (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133109/new/ http

  1   2   3   4   >