[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] D126451: [Clang][CSKY] Add support about CSKYABIInfo

DavidSpickett added a comment. Can you reupload with more context? See https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface I doubt I'll be able to give any in depth review but if things look good generally I'm sure you'll find the issues through your own testing as

[PATCH] D126451: [Clang][CSKY] Add support about CSKYABIInfo

DavidSpickett added a comment. Looks good to me. Can you just clarify how the tests are split? My guess is that one is stuff that doesn't vary with hard/soft float and the other is the bits that change when hardfloat is enabled. If so is it worth checking those situations with soft float too or

[PATCH] D126451: [Clang][CSKY] Add support about CSKYABIInfo

DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126451/new/ https://reviews.llvm.org/D126451 ___ cfe-commits mailing list

[PATCH] D127093: [clang][pseudo] Add missing Support lib to cxx

DavidSpickett created this revision. Herald added subscribers: jsji, pengfei, mgorny. Herald added a project: All. DavidSpickett requested review of this revision. Herald added subscribers: cfe-commits, alextsao1999. Herald added a project: clang-tools-extra. Without this the following build confi

[PATCH] D127093: [clang][pseudo] Add missing Support lib to cxx

DavidSpickett added reviewers: sammccall, MaskRay. DavidSpickett added a comment. Herald added a subscriber: StephenFan. I'm basing this on what `psuedo/lib/grammar` does, so I'm not totally sure this is the place to fix it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D127093: [clang][pseudo] Add missing Support lib to cxx

DavidSpickett added a comment. `DBUILD_SHARED_LIBS` being the key setting in the config that causes the issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127093/new/ https://reviews.llvm.org/D127093 _

[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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] D69574: Remove lazy thread-initialisation

This revision was automatically updated to reflect the committed changes. Closed by commit rG91167e22eca5: [hwasan] Remove lazy thread-initialisation (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69574/new/ https://revie

[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] 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. > 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] 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] 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] 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 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] 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] 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] 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: [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: [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] 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

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

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

[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

[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] 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] 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] 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] 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] 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] 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] 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 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

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] 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] 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. 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 `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 `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 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] 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] 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] 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] 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] 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] 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] 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 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 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 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] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

DavidSpickett added a comment. Hi, this patch has caused a gcc test suite failure on our SVE bots: https://lab.llvm.org/buildbot/#/builders/184/builds/1941 (test source is https://github.com/llvm/llvm-test-suite/blob/main/SingleSource/Regression/C/gcc-c-torture/execute/pr79286.c) I've only just

[PATCH] D118044: [ARM] Undeprecate complex IT blocks

DavidSpickett added a comment. Please define what "complex" is, in the commit message and (if it doesn't make the description a whole paragraph) the command line help. Given that we're flipping a default for v8 thumb targets does this warrant a release note? Comment at: llvm

[PATCH] D118044: [ARM] Undeprecate complex IT blocks

DavidSpickett added a comment. I may have missed this in the many test changes but are we left with a test that demonstrates that the codegen restriction still works? I see some that set one or the other, any with both that would catch us breaking this feature later? Repository: rG LLVM Gith

[PATCH] D118044: [ARM] Undeprecate complex IT blocks

DavidSpickett added inline comments. Comment at: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp:10977 -Warning(IDLoc, "deprecated instruction in IT block"); - } } MarkMurrayARM wrote: > DavidSpickett wrote: > > Is this warning something you'd st

[PATCH] D118044: [ARM] Undeprecate complex IT blocks

DavidSpickett added inline comments. Comment at: llvm/docs/ReleaseNotes.rst:87 * Added support for the Armv8.1-M PACBTI-M extension. +* Removed the deprecation of ARMv8-A T32 Complex IT blocks. Such blocks + are now enabled by default, but may be turned off if required. No

[PATCH] D118044: [ARM] Undeprecate complex IT blocks

DavidSpickett added a comment. FWIW, a 2 stage build on armv8 with `-mthumb` didn't turn up any codegen Gremlins. (that's how I saw the unused var) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118044/new/ https://reviews.llvm.org/D118044 ___

[PATCH] D121206: [AARCH64] ssbs should be enabled by default for cortex-x1c

DavidSpickett added inline comments. Comment at: clang/test/Preprocessor/aarch64-target-features.c:288 // CHECK-MCPU-CORTEX-A73: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" "-target-feature" "+crc" "-t

[PATCH] D100772: [ARM] Neon Polynomial vadd Intrinsic fix

DavidSpickett added a comment. I'd merge https://reviews.llvm.org/D100499 into this given that it's only one line. Comment at: clang/include/clang/Basic/arm_neon.td:712 + +// Crypto +// TODO: poly1

[PATCH] D100772: [ARM] Neon Polynomial vadd Intrinsic fix

DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100772/new/ https://reviews.llvm.org/D100772 ___ cfe-commits mailing list

[PATCH] D100919: [AArch64] Support customizing stack protector guard

DavidSpickett added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3110 +} +if (EffectiveTriple.isAArch64() && Value != "sysreg" && Value != "global") { + D.Diag(diag::err_drv_invalid_value_with_suggestion) Shouldn't this also al

[PATCH] D101327: [Clang][Driver] validate sysregs for -mstack-protector-guard-reg=

DavidSpickett added a comment. A lot of these system registers are going to be unsuitable for the stack canary. They're either read only, they have fixed bits, or they'll get overwritten by various system events. I'm not suggesting we audit this list for those things, what seems reasonable to

[PATCH] D100919: [AArch64] Support customizing stack protector guard

DavidSpickett added inline comments. Comment at: clang/test/Driver/stack-protector-guard.c:59 +// INVALID-VALUE-AARCH64: error: invalid value 'tls' in 'mstack-protector-guard=','valid arguments to '-mstack-protector-guard=' are:sysreg global' +// INVALID-REG-AARCH64: error: inv

[PATCH] D100919: [AArch64] Support customizing stack protector guard

DavidSpickett added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.h:385 /// On x86 this can be "fs" or "gs". + /// On AArch64 this can be any value of llvm::AArch64SysReg::SysReg. std::string StackProtectorGuardReg; This is now used

[PATCH] D100919: [AArch64] Support customizing stack protector guard

DavidSpickett added a reviewer: ostannard. DavidSpickett added a subscriber: ostannard. DavidSpickett added a comment. All my comments have been answered. Perhaps @ostannard can check AArch64InstrInfo.cpp ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D101849: [OpenMP][NFC] Refactor Clang OpenMP tests using update_cc_test_checks

DavidSpickett added a comment. @ggeorgakoudis `Clang::parallel_for_codegen.cpp` is failing on some of our bots: https://lab.llvm.org/buildbot/#/builders/43/builds/5751 https://lab.llvm.org/buildbot/#/builders/107/builds/7585 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D101849: [OpenMP][NFC] Refactor Clang OpenMP tests using update_cc_test_checks

DavidSpickett added a comment. The failing test line is: // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -O1 -fopenmp-simd -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK10 It passes if you have the X86 backend enabled but fails if you do not. Our bots usually have only Arm o

[PATCH] D101849: [OpenMP][NFC] Refactor Clang OpenMP tests using update_cc_test_checks

DavidSpickett added a comment. I've required X86 target for this test to get our bots green again. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101849/new/ https://reviews.llvm.org/D101849 ___ cfe-commi

[PATCH] D101952: [OpenMP] Fix non-determinism in clang copyin codegen

DavidSpickett added a comment. Thanks! I'll test our Arm build and revert the requires once I've confirmed it passes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101952/new/ https://reviews.llvm.org/D101952 _

[PATCH] D100919: [AArch64] Support customizing stack protector guard

DavidSpickett requested changes to this revision. DavidSpickett added a comment. This revision now requires changes to proceed. This is probably not showing up in review queues as it got accepted accidentally at some point. I'm going to "Request Changes" then I think you'll need to refresh the d

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

DavidSpickett added a comment. @ychen Since this change the warning emitted in `AsmPrinterInlineAsm.cpp` about reserved registers no longer has location information for the file that the inline asm is in. Here's an example: void bar(void) { __asm__ __volatile__ ( "nop" : : : "sp");

[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

DavidSpickett added a comment. Can you guide me on how I might correct this? Actually, now I see that MCContext has reportWarning/reportError and just needs a reportNote. I'll have something for you to review shortly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D102244: [llvm][AsmPrinter] Restore source location to register clobber warning

DavidSpickett created this revision. Herald added a subscriber: hiraditya. DavidSpickett requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Since 5de2d189e6ad466a1f0616195e8c524a4eb3cbc0

[PATCH] D102244: [llvm][AsmPrinter] Restore source location to register clobber warning

DavidSpickett added a reviewer: ychen. DavidSpickett added a comment. This works as expected, let me know if this is the intended use relative to the changes you made. The existing test in llvm `llvm/test/CodeGen/AArch64/inline-asm-clobber.ll` never reported the location in the .ll file so I ca

[PATCH] D112427: [ARM] Implement setjmp BTI placement for PACBTI-M

DavidSpickett added inline comments. Comment at: llvm/lib/Target/ARM/ARMInstrThumb2.td:5745 + IIC_Br, [(ARMt2CallBTI tglobaladdr:$func)]>, + Requires<[IsThumb2]>, Sched<[WriteBrL]>; Should this require `IsMClass` instead/also? Thou

[PATCH] D112427: [ARM] Implement setjmp BTI placement for PACBTI-M

DavidSpickett added inline comments. Comment at: llvm/lib/Target/ARM/ARMInstrThumb2.td:5745 + IIC_Br, [(ARMt2CallBTI tglobaladdr:$func)]>, + Requires<[IsThumb2]>, Sched<[WriteBrL]>; chill wrote: > DavidSpickett wrote: > > Should th

[PATCH] D112427: [ARM] Implement setjmp BTI placement for PACBTI-M

DavidSpickett added inline comments. Comment at: llvm/lib/Target/ARM/ARMInstrThumb2.td:5745 + IIC_Br, [(ARMt2CallBTI tglobaladdr:$func)]>, + Requires<[IsThumb2]>, Sched<[WriteBrL]>; DavidSpickett wrote: > chill wrote: > > DavidSpic

[PATCH] D112091: libfuzzer: All building libfuzzer for ARM32

DavidSpickett added a comment. Since this change we've had check-2 timeouts on our 2 stage v7 bots https://lab.llvm.org/buildbot/#/builders/190/builds/513 (we have one building thumb and one Arm) These bots run on v7 hardware as opposed to some of our others that actually run on v8 hardware, t

[PATCH] D112091: libfuzzer: All building libfuzzer for ARM32

DavidSpickett added a comment. Done in https://reviews.llvm.org/rG6bfbb89e96fa. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112091/new/ https://reviews.llvm.org/D112091 ___ cfe-commits mailing list cfe

[PATCH] D115320: Avoid setting tbaa information on store of return type of call to inline assember

DavidSpickett added a comment. Hi, the new test is failing on our arm/aarch64 quick bots. These build only the respective backend so you might need to add a requires x86 target. https://lab.llvm.org/buildbot/#/builders/171/builds/7189 /home/tcwg-buildbot/worker/clang-armv7-quick/llvm/clang/t

[PATCH] D92270: [ConstantFold] Fold more operations to poison

DavidSpickett added a comment. I bisected an arm 32 bit issue with WebKit Javascript's infinity value (https://github.com/llvm/llvm-project/issues/52669) down to this commit. Just to make people aware there is another failing case. No reproducer yet I'm still working on a way to reduce it, but

[PATCH] D115320: Avoid setting tbaa information on store of return type of call to inline assember

DavidSpickett added a comment. I've gone ahead and added the REQUIRES myself. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115320/new/ https://reviews.llvm.org/D115320 ___ cfe-commits mailing list cfe-c

[PATCH] D99250: [DebugInfo] Fix the mismatching of C++ language tags and Dwarf versions.

DavidSpickett added a comment. This change has caused a test failure in lldb. On our Arm bots: http://lab.llvm.org:8011/#/builders/96/builds/6582 Also on Linux x86_64: http://lab.llvm.org:8011/#/builders/68/builds/10277 Not windows though because `test/API/commands/frame/language/TestGuessLangu

[PATCH] D99250: [DebugInfo] Fix the mismatching of C++ language tags and Dwarf versions.

DavidSpickett added a subscriber: omjavaid. DavidSpickett added a comment. Ah, @omjavaid already put in a fix. I think you can just reland the change now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99250/new/ https://reviews.llvm.org/D99250 ___

[PATCH] D100499: [AArch64] Neon Polynomial vadd Intrinsic Fix

DavidSpickett added a comment. What's your logic for these being Arm only? I looked up the ones that were added: vadd_p8 vadd_p16 vadd_p64 vaddq_p8 vaddq_p16 vaddq_p64 vaddq_p128 E.g. https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics?search=vadd_p

[PATCH] D100499: [AArch64] Neon Polynomial vadd Intrinsic Fix

DavidSpickett added a comment. Both clang and GCC have their issues when it comes to matching the ACLE, so I wouldn't take the header guard as fact. It could be that we never implemented the A32 path for these functions/when they were added the document was in flux/no on ever tried this on A32.

<    1   2   3   4   >