[PATCH] D132531: [AArch64] Reserve more physical registers

kristof.beyls added a comment. See https://reviews.llvm.org/D56305 and https://reviews.llvm.org/D48580 for previous related discussions. I think it would be helpful to understand the use case for being able to reserve x8, x16, x17 and x19 better. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D132531: [AArch64] Reserve more physical registers

kristof.beyls added a comment. My understanding is that X8, X16, X17 and X19 cannot be reserved because the code generator in places will make use of them. For example, using X19 as a base register in some cases. X16 and X17 are defined by the ABI to potentially be clobbered on function calls &

[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

kristof.beyls added a comment. See https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html, documentation for "mharden-sls": For AArch64, the options available on the command line are "retbr", "blr", "none" and "all". I don't think the options necessarily have to be the same for x86. But assumin

[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

kristof.beyls added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:752 +#undef CASE + } +} Just a drive-by comment: I'm wondering if SVE registers should also be listed here? Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D113942: [NFC][clang] Inclusive language: replace master with main in convert_arm_neon.py

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

[PATCH] D112941: [clang] Add support for the new pointer authentication builtins.

kristof.beyls added inline comments. Comment at: clang/include/clang/Driver/Options.td:2865-2872 +let Group = f_Group in { + let Flags = [CC1Option] in { +def fptrauth_intrinsics : Flag<["-"], "fptrauth-intrinsics">, + HelpText<"Enable pointer-authentication intrinsics"

[PATCH] D112941: [clang] Add support for the new pointer authentication builtins.

kristof.beyls added inline comments. Comment at: clang/lib/Headers/ptrauth.h:19-37 + /* A process-independent key which can be used to sign code pointers. + Signing and authenticating with this key is a no-op in processes + which disable ABI pointer authentication. */ +

[PATCH] D112941: [clang] Add support for the new pointer authentication builtins.

kristof.beyls added inline comments. Comment at: clang/include/clang/Driver/Options.td:2865-2872 +let Group = f_Group in { + let Flags = [CC1Option] in { +def fptrauth_intrinsics : Flag<["-"], "fptrauth-intrinsics">, + HelpText<"Enable pointer-authentication intrinsics"

[PATCH] D111134: Add basic aarch64-none-elf bare metal driver.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3b93dc6880f7: Add basic aarch64-none-elf bare metal driver. (authored by kristof.beyls). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D111134: Add basic aarch64-none-elf bare metal driver.

kristof.beyls added a comment. @MaskRay - gentle ping: I wonder if you have any further remarks after I updated the patch based on your earlier feedback? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D34/new/ https://reviews.llvm.org/D34

[PATCH] D111134: Add basic aarch64-none-elf bare metal driver.

kristof.beyls updated this revision to Diff 377818. kristof.beyls added a comment. run clang-format on the patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D34/new/ https://reviews.llvm.org/D34 Files: clang/lib/Driver/ToolChains/BareMetal.cpp clang/test/Driver/baremetal.

[PATCH] D111134: Add basic aarch64-none-elf bare metal driver.

kristof.beyls added inline comments. Comment at: clang/lib/Driver/ToolChains/BareMetal.cpp:133 + + if (Triple.getVendor() != llvm::Triple::UnknownVendor) +return false; MaskRay wrote: > Is vendor check necessary? I'm guessing that without the vendor check, t

[PATCH] D111134: Add basic aarch64-none-elf bare metal driver.

kristof.beyls updated this revision to Diff 377816. kristof.beyls added a comment. Updated test based on feedback from @MaskRay CHANGES SINCE LAST ACTION https://reviews.llvm.org/D34/new/ https://reviews.llvm.org/D34 Files: clang/lib/Driver/ToolChains/BareMetal.cpp clang/test/Dri

[PATCH] D111134: Add basic aarch64-none-elf bare metal driver.

kristof.beyls created this revision. kristof.beyls added reviewers: psmith, miyuki, srhines. Herald added subscribers: s.egerton, simoncook. kristof.beyls requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

[PATCH] D103080: [CMake] Ignore arm_*.h for non-ARM build

kristof.beyls added a comment. In D103080#2790139 , @sunlin wrote: > Hi @kristof.beyls The original `lib/clang/12.0.1/include` is about total > ~10M, and the `arm*.h` take about ~5M. Ignore these unused header files will > save the developers who work

[PATCH] D103080: [CMake] Ignore arm_*.h for non-ARM build

kristof.beyls added a comment. I'm wondering what the rationale for this change is. If there is a good rationale for this; wouldn't it need to be applied to all target-specific header files, not only the Arm-specific header files? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

kristof.beyls added inline comments. Comment at: clang/test/Driver/stack-protector-guard.c:26 -// RUN: not %clang -target aarch64-linux-gnu -mstack-protector-guard-offset=10 %s 2>&1 | \ +// RUN: not %clang -target arm-linux-gnuebi -mstack-protector-guard-offset=10 %s 2>&1 | \

[PATCH] D93347: [Test] Fix undef var in attr-speculative-load-hardening.c

kristof.beyls accepted this revision. kristof.beyls added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93347/new/ https://reviews.llvm.org/D93347 __

[PATCH] D98277: [release][docs] List all cores Arm has added support for in LLVM 12.

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

[PATCH] D98277: [release][docs] List all cores Arm has added support for in LLVM 12.

kristof.beyls accepted this revision. kristof.beyls added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98277/new/ https://reviews.llvm.org/D98277 __

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

[PATCH] D93221: [ARM] Add clang command line support for -mharden-sls=

kristof.beyls added a comment. In D93221#2459903 , @ostannard wrote: > Why is this restricted to v7-A or later? The DSB and ISB instructions have > existed since v6T2 and v6M. This mitigation is never needed for M-class cores nor for v6T2. By restricting

[PATCH] D93221: [ARM] Add clang command line support for -mharden-sls=

kristof.beyls created this revision. kristof.beyls added a reviewer: ostannard. Herald added a subscriber: danielkiss. kristof.beyls requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The command line syntax is identical to the -mharden-sls= c

[PATCH] D92245: -fstack-clash-protection: Return an actual error when used on unsupported OS

kristof.beyls added inline comments. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:276-277 "-fembed-bitcode is not supported on versions of iOS prior to 6.0">; +def err_drv_stack_clash_protection_unsupported_on_toolchain : Error< + "-fstack-clash-protection i

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

kristof.beyls accepted this revision. kristof.beyls added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76291/new/ https://reviews.llvm.org/D76291

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

kristof.beyls added a comment. Herald added a project: LLVM. This looks fine to me; I just have a number of nit picks. The only part where I don't understand the code logic is around the comment starting with "If this is the final byte of a multi-byte sequence". Comment at: ll

[PATCH] D81404: [AArch64] Add clang command line support for -mharden-sls=

kristof.beyls marked 2 inline comments as done. kristof.beyls added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:229 + Scope.split(Opts, ","); + for (int I = 0, E = Opts.size(); I != E; ++I) { +StringRef Opt = Opts[I].trim(); ---

[PATCH] D81404: [AArch64] Add clang command line support for -mharden-sls=

This revision was automatically updated to reflect the committed changes. Closed by commit rGc113b59ef525: [AArch64] Add clang command line support for -mharden-sls= (authored by kristof.beyls). Changed prior to commit: https://reviews.llvm.org/D81404?vs=269247&id=271925#toc Repository: rG L

[PATCH] D81404: [AArch64] Add clang command line support for -mharden-sls=

kristof.beyls created this revision. kristof.beyls added a reviewer: ostannard. Herald added subscribers: cfe-commits, danielkiss. Herald added a project: clang. The accepted options to -mharden-sls= are: - all: enable all mitigations against Straight Line Speculation that are implemented. - non

[PATCH] D78481: [ARM] Release notes for the Custom Datapath Extension (CDE)

kristof.beyls accepted this revision. kristof.beyls 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/D78481/new/ https://reviews.llvm.org/D78481 _

[PATCH] D76513: [ReleaseNotes,ARM] MVE intrinsics are all implemented!

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

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

kristof.beyls added a comment. In D74918#1923151 , @zoecarver wrote: > There are a lot of different ways we could implement the feature. We may want > to only enable it when `-march=native`, or maybe only in the unstable ABI, > and maybe we want to suppo

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

kristof.beyls added a comment. In D74918#1898191 , @__simt__ wrote: > In D74918#1897636 , @kristof.beyls > wrote: > > > If these values are part of the C++ target platform ABI, it seems to me the > > values for st

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

kristof.beyls added a comment. In D74918#1896930 , @__simt__ wrote: > (//I assume I'm not seeing a code review being used to veto a C++ Standard > feature, but actually the other points are the reason for the red flag.//) > > I can see a desire for hyper-

[PATCH] D72449: [PATCH] [llvm-ranlib] Take in consideration UTC offset for D-flag.test

kristof.beyls added inline comments. Comment at: llvm/test/tools/llvm-ranlib/D-flag.test:13 # RUN: cp %t-no-index.a %t.a && llvm-ranlib -D %t.a -# RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s --check-prefix=DETERMINISTIC-VALUES +# RUN: (env TZ=UTC date -d '@0' +%H:%M; env TZ=

[PATCH] D70779: AArch64: add support for newer Apple CPUs

kristof.beyls added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:143 MtuneLowerCase = llvm::sys::getHostCPUName(); - if (MtuneLowerCase == "cyclone") { + if (MtuneLowerCase == "cyclone" || MtuneLowerCase.find("apple") == 0) { Features.push

[PATCH] D70779: AArch64: add support for newer Apple CPUs

kristof.beyls added a comment. I agree the bare names could cause a lot of confusion, and that the naming scheme proposed in this patch resolves that potential confusion. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70779/new/ https://reviews.llvm.org/D70779 ___

[PATCH] D67678: PR17164: Change clang's default behavior from -flax-vector-conversions=all to -flax-vector-conversions=integer.

kristof.beyls added inline comments. Comment at: docs/ReleaseNotes.rst:84 + In a future release of Clang, we intend to change the default to + ``-fno-lax-vector-conversions``. + rsmith wrote: > efriedma wrote: > > kristof.beyls wrote: > > > efriedma wrote: > >

[PATCH] D67678: PR17164: Change clang's default behavior from -flax-vector-conversions=all to -flax-vector-conversions=integer.

kristof.beyls added inline comments. Comment at: docs/ReleaseNotes.rst:84 + In a future release of Clang, we intend to change the default to + ``-fno-lax-vector-conversions``. + efriedma wrote: > rsmith wrote: > > efriedma wrote: > > > And if you want to allow

[PATCH] D67160: [clang, ARM] Default to -fno-lax-vector-conversions in ARM v8.1-M.

kristof.beyls added a comment. Would it still make sense to have this patch after D68683 lands? At first sight, it seems this patch might no longer make sense then? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

kristof.beyls added inline comments. Comment at: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll:1-6 +; RUN: llc -mtriple=armv7a-linux-gnueabihf %s -o - | FileCheck %s --check-prefix=CHECK-ARM +; RUN: llc -mtriple=armv7a-linux-gnueabihf %s -o - | FileCheck %s --check-prefix=CHECK-ARM-FA

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

kristof.beyls added a comment. I've just added a few fly-by nits; I'm afraid I didn't do an in-depth review. Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1156 MachineInstr &MI = *MBBI; + LLVM_DEBUG(dbgs() << "ARMExpandPseudo::ExpandMI: " << MI << "\n"); unsig

[PATCH] D59827: [slh] x86 impl of ARM instrinsic + builtin for SLH

kristof.beyls added a comment. This intrinsic got added to gcc a while ago and should become available in the upcoming gcc 9 release. In gcc however, the prototype of the intrinsic is slightly different (see https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html): type __builtin_speculation_safe

[PATCH] D59827: [slh] x86 impl of ARM instrinsic for SLH

kristof.beyls added a comment. Thanks for picking this up, Zola! I quickly looked through the patch - comparing it with what I had done under D49070 and D49073 . Apart from the point remarks inline, I had the following immediate

[PATCH] D53121: [Driver] Add defaults for Android ARM FPUs.

kristof.beyls added inline comments. Comment at: test/Driver/arm-mfpu.c:410 + +// RUN: %clang -target armv7-linux-androideabi23 %s -mfpu=vfp3-d16 -### -c 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-ARM-ANDROID-M-FP-D16 %s danalbert wrote: > >>! In D53121#

[PATCH] D53121: [Driver] Add defaults for Android ARM FPUs.

kristof.beyls added a subscriber: peter.smith. kristof.beyls added a comment. In https://reviews.llvm.org/D53121#1261408, @srhines wrote: > This LGTM, but we should wait to hear from Kristof before submitting. Seems fine to me too. I'd maybe just add an additional test case to verify that thin

[PATCH] D51157: [x86/SLH] Add a real Clang flag and LLVM IR attribute for Speculative Load Hardening.

kristof.beyls accepted this revision. kristof.beyls added a comment. I'm not an expert on many of the areas touched by this patch, but it looks fine from me from a high-level point-of-view, modulo a few nits I have on a few comments. Comment at: clang/lib/Driver/ToolChains/Ar

[PATCH] D51157: [x86/SLH] Add a real Clang flag and LLVM IR attribute for Speculative Load Hardening.

kristof.beyls added inline comments. Comment at: llvm/docs/LangRef.rst:1659-1661 +that hardening. It should also be possible to *not* harden a hot and/or safe +function and have code inlined there *not* be hardened (even if the generic +form is hardened). ---

[PATCH] D49073: Introducing __builtin_speculation_safe_value

kristof.beyls created this revision. Herald added subscribers: cfe-commits, aheejin, dschuff. This is part of implementing a technique to mitigate against Spectre v1, similar in spirit to what has been proposed by Chandler for X86_64 at http://lists.llvm.org/pipermail/llvm-dev/2018-March/122085.ht

[PATCH] D41760: Introduce __builtin_load_no_speculate

kristof.beyls added a comment. The API design has been discussed over the past weeks in detail on the gcc mailing list. As a result of that, we propose to adapt the API, to enable efficient code generation also on architectures that need to generate a barrier instruction to achieve the desired

[PATCH] D41760: Introduce __builtin_load_no_speculate

kristof.beyls created this revision. kristof.beyls added a reviewer: olista01. Herald added subscribers: aheejin, javed.absar, dschuff, jfb, aemerson. Recently, Google Project Zero disclosed several classes of attack against speculative execution. One of these, known as variant-1 (CVE-2017-5753),

[PATCH] D34878: [ARM] Option for reading thread pointer from coprocessor register

kristof.beyls added a comment. Still LGTM; please commit. https://reviews.llvm.org/D34878 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34878: [ARM] Option for reading thread pointer from coprocessor register

kristof.beyls accepted this revision. kristof.beyls added a comment. This revision is now accepted and ready to land. Thanks Strahinja! I thought that some indentations looked a bit strange, so I'd just still check that clang-format formats your changes the same way. Otherwise LGTM! https://rev

[PATCH] D34878: [ARM] Option for reading thread pointer from coprocessor register

kristof.beyls added inline comments. Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:137 + const Driver &D = TC.getDriver(); + arm::ReadTPMode ThreadPointer = ReadTPMode::Invalid; + if (Arg *A = With the new version of the code, there's no need to have a Thread

[PATCH] D34878: [ARM] Option for reading thread pointer from coprocessor register

kristof.beyls added inline comments. Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:145-146 + // choose soft mode. + if (ThreadPointer == ReadTPMode::Invalid) +ThreadPointer = ReadTPMode::Soft; + return ThreadPointer; spetrovic wrote: > kristof.beyls wrote

[PATCH] D34878: [ARM] Option for reading thread pointer from coprocessor register

kristof.beyls added inline comments. Comment at: include/clang/Driver/Options.td:1664-1665 HelpText<"Allow generation of data access to code sections (ARM only)">; +def mtp_mode_EQ : Joined<["-"], "mtp=">, Group, Values<"soft, cp15">, + HelpText<"Read thread pointer from cop