[PATCH] D156784: [AArch64][PAC] Declare FPAC subtarget feature

chill added a comment. In D156784#4653741 , @atrosinenko wrote: > As discussed in D156716 , it is not clear > if I have to add FeatureFPAC to every relevant CPU. I would say, yes, it has to be added to each CPU that ha

[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs passed as function arguments

chill added inline comments. Comment at: clang/test/CodeGen/aarch64-ABI-align-packed-assembly.c:100 struct packed_struct on_callee_stack; on_callee_stack = va_arg(vl, struct packed_struct); } Can we add some `CHECK:` lines here and to other variadic functi

[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs passed as function arguments

chill added a comment. Following D148094 , the patch does not apply. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146242/new/ https://reviews.llvm.org/D146242 ___ cfe-

[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs passed as function arguments

chill added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:5813 getContext().getTypeUnadjustedAlignInChars(Ty).getQuantity(); -unsigned BaseAlign = getContext().getTypeAlignInChars(Base).getQuantity(); -Align = (Align > BaseAlign && Align >= 16) ?

[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs passed as function arguments

chill added a comment. > Previously when a packed struct, containing vector data types such as > uint16x8_t, is passed as a function argument, the alignment of the > struct used by the function caller and the alignment used by the callee > to load the argument from stack does not match. I would s

[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs passed as function arguments

chill added a comment. In D146242#4427966 , @tmatheson wrote: > In D146242#4427707 , @chill wrote: > >> I was just thinking to LGTM it :) >> >> IMHO, the alignment adjustment happens because of C.4 (B.3 indeed le

[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs passed as function arguments

chill added inline comments. Comment at: clang/test/CodeGen/aarch64-ABI-align-packed.c:34 struct aligned_member_8 { uint16x8_t M0 __attribute((aligned (8))); // member alignment 16 since __attribute((aligned (n))) sets the minimum alignment }; JiruiWu wrote

[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs passed as function arguments

chill added a comment. I was just thinking to LGTM it :) IMHO, the alignment adjustment happens because of C.4 (B.3 indeed leave the HFA/HVA unmodified). > C.4 If the argument is an HFA, an HVA, a Quad-precision Floating-point or > short vector type then the NSAA is rounded up to the next m

[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs passed as function arguments

chill added inline comments. Comment at: clang/test/CodeGen/aarch64-ABI-align-packed.c:34 struct aligned_member_8 { uint16x8_t M0 __attribute((aligned (8))); // member alignment 16 since __attribute((aligned (n))) sets the minimum alignment }; Don't you mea

[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs passed as function arguments

chill added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:5809 -// For alignment adjusted HFAs, cap the argument alignment to 16, leave it -// default otherwise. +// For alignment adjusted HFAs, cap the argument alignment to 16, otherwise +// set

[PATCH] D123498: [clang] Adding Platform/Architecture Specific Resource Header Installation Targets

chill added a comment. Should these lists contain only source tree headers or also generated header files? I'm not seeing `arm_mve.h`, for example. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123498/new/ https://reviews.llvm.org/D123498 ___

[PATCH] D114543: Extend the `uwtable` attribute with unwind table kind

chill added inline comments. Comment at: llvm/lib/IR/Attributes.cpp:453 +return "uwtable"; + return ("uwtable(" + Twine(Kind == UWTableKind::Sync ? "sync" : "async") + + ")") RKSimon wrote: > @chill Static analysis is warning that its i

[PATCH] D114543: Extend the `uwtable` attribute with unwind table kind

chill added a comment. In D114543#3319587 , @durin42 wrote: > > Is the parameter optional if uwtable is set programmatically, or only when > we're reading IR streams? No, it's not optional, the attribute is added by https://github.com/llvm/llvm-pro

[PATCH] D114543: Extend the `uwtable` attribute with unwind table kind

chill added a comment. In D114543#3319347 , @durin42 wrote: > As far as I can tell this patch broke the Rust compiler, but from the commit > message it sounds like it shouldn't have? > > https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/bu

[PATCH] D119724: Fix test failure for targets with varying uwtable defaults

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa31d00ddceb0: Fix test failure for targets with varying uwtable defaults (authored by chill). Herald added a project: clang. Herald added a subscribe

[PATCH] D119166: [clang][ARM] Re-word PACBTI warning.

chill added inline comments. Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:149 +def warn_incompatible_branch_protection_option: Warning < + "'-mbranch-protection=' option incompatible with the '%0' architecture">, InGroup; "is incompatible"

[PATCH] D118199: [AArch64] ACLE feature macro for Armv8.8-A MOPS

chill added a comment. In D118199#3287636 , @chill wrote: > I suppose `clang/test/CodeGen/aarch64-mops.c` needs to be run with `clang > -march=armv8-a+mops+memtag` (not `clang_cc1`) so it picks up declarations of > tagging intrinsics from `arm_acle.h`.

[PATCH] D118199: [AArch64] ACLE feature macro for Armv8.8-A MOPS

chill added a comment. I suppose `clang/test/CodeGen/aarch64-mops.c` needs to be run with `clang -march=armv8-a+mops+memtag` (not `clang_cc1`) so it picks up declarations of tagging intrinsics from `arm_acle.h`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D118199: [AArch64] ACLE feature macro for Armv8.8-A MOPS

chill added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:229 MacroBuilder &Builder) const { + // FIXME: this does not handle the case where MOPS is disabled using +nomops + Builder.defineMacro("__ARM_FEATURE_M

[PATCH] D115501: [clang][ARM] Emit warnings when PACBTI-M is used with unsupported architectures

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

[PATCH] D115501: [clang][ARM] Emit warnings when PACBTI-M is used with unsupported architectures

chill added inline comments. Comment at: clang/lib/Basic/Targets/ARM.cpp:387 + + return a.isArmT32(); +} For example Arm7-a defines the T32 instruction set, buy we still want a warning. Maybe we need `return a.isArmT32() && a.isArmMClass()`. I'm not actually su

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

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

[PATCH] D116160: [AArch64] ACLE feature macro for Armv8.8-A MOPS

chill added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:666 + HasMOPS |= ArchKind == llvm::AArch64::ArchKind::ARMV8_8A || + ArchKind == llvm::AArch64::ArchKind::ARMV9_3A; So, this is enabled by default (as in "is mandatory part

[PATCH] D115694: [ARM] Introduce an empty "armv8.8-a" architecture.

chill added inline comments. Comment at: llvm/include/llvm/Support/ARMTargetParser.def:125 ARM::AEK_DOTPROD| ARM::AEK_BF16 | ARM::AEK_I8MM)) +ARM_ARCH("armv8.8-a", ARMV8_8A, "8.8-A", "v8.8a", + ARMBuildAttrs::CPUArch::v8_A, FK_CRYPTO_NEON_FP_ARMV8,

[PATCH] D115501: [clang][ARM] Emit warnings when PACBTI-M is used with unsupported architectures

chill added inline comments. Comment at: clang/lib/Basic/Targets/ARM.cpp:391-392 + if (!Arch.empty() && !isBranchProtectionSupportedArch(Arch)) +return false; + amilendra wrote: > chill wrote: > > On empty `Arch` it'd continue down the function, but we'd l

[PATCH] D115501: [clang][ARM] Emit warnings when PACBTI-M is used with unsupported architectures

chill added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.h:70 - bool validateBranchProtection(StringRef, BranchProtectionInfo &, + bool validateBranchProtection(StringRef, StringRef, BranchProtectionInfo &, StringRef &) const o

[PATCH] D112421: [clang][ARM] PACBTI-M frontend support

chill added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:6377 + +static const char *SignReturnAddrStr[] = {"none", "non-leaf", "all"}; +Fn->addFnAttr("sign-return-address", vhscampos wrote: > I reckon selecting the string using a

[PATCH] D112430: [ARM][libunwind] add PACBTI-M support for libunwind

chill added inline comments. Comment at: libunwind/src/Unwind-EHABI.cpp:312 uint8_t registers = getByte(data, offset++); - if (registers & 0xf0 || !registers) + if (registers & 0xf0) return _URC_FAILURE; m

[PATCH] D112431: [ARM][clang] Define feature test macro for the PACBTI-M extension

chill added inline comments. Comment at: clang/lib/Basic/Targets/ARM.cpp:897 +Builder.defineMacro("__ARM_FEATURE_PAUTH", "1"); +Builder.defineMacro("__ARM_FEATURE_BTI", "1"); + } miyuki wrote: > Since we decided to have two separate flags, I think this l

[PATCH] D112421: [clang][ARM] PACBTI-M frontend support

chill added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:134-135 StringRef &Err) const { - llvm::AArch64::ParsedBranchProtection PBP; - if (!llvm::AArch64::parseBranchProtection(Spec, PBP, Err)) + llvm::ARM:

[PATCH] D112421: [clang][ARM] PACBTI-M frontend support

chill added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:134-135 StringRef &Err) const { - llvm::AArch64::ParsedBranchProtection PBP; - if (!llvm::AArch64::parseBranchProtection(Spec, PBP, Err)) + llvm::ARM:

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

chill added a comment. IMHO, it's possible to write a frontend test, which includes, say `arm_neon.h`, but does not really require the `ARM` or `AArch64` backends to be configured (e.g. `CodeGen/arm-vector-align.c`?) If `arm_neon.h` is not built, then the test would need the appropriate `REQUIR

[PATCH] D102693: Do not create LLVM IR `constant`s for objects with dynamic initialisation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG21aa107eb79f: Reland "Do not create LLVM IR `constant`s for objects with dynamic… (authored by chill). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D102693: Do not create LLVM IR `constant`s for objects with dynamic initialisation

chill updated this revision to Diff 347631. chill added a comment. This revision is now accepted and ready to land. Updated a test to run for `x86_64-linux` instead of `%itanium_abi_triple`, to avoid having invalid syntax for MACH-O sections. The patch itself does not care about section attribut

[PATCH] D102693: Do not create LLVM IR `constant`s for objects with dynamic initialisation

chill added a comment. Thanks, I'll have a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102693/new/ https://reviews.llvm.org/D102693 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:/

[PATCH] D102693: Do not create LLVM IR `constant`s for objects with dynamic initialisation

This revision was automatically updated to reflect the committed changes. Closed by commit rG13dd65b3a1a3: Do not create LLVM IR `constant`s for objects with dynamic initialisation (authored by chill). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Gi

[PATCH] D100853: [clang][AArch32] Correctly align HA arguments when passed on the stack

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5c7b43aa8298: [clang][AArch32] Correctly align HA arguments when passed on the stack (authored by chill). Herald added a project: clang. Herald added

[PATCH] D98794: [AArch64] Correctly align HFA arguments when passed on the stack

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf9d932e6735a: [clang][AArch64] Correctly align HFA arguments when passed on the stack (authored by chill). Herald added a project: clang. Herald adde

[PATCH] D75903: [AArch64][CodeGen] Fixing stack alignment of HFA arguments on AArch64 PCS

chill added inline comments. Comment at: llvm/docs/LangRef.rst:1220 +``alignstack()`` +This indicates the alignment that should be considered by the backend when +assigning this parameter to a stack slot during calling convention rnk wrote: > This seems l

[PATCH] D94083: [AArch64] Add +pauth archictecture option, allowing the v8.3a pointer authentication extension.

chill added a comment. In D94083#2486800 , @fhahn wrote: > FWIW I think it would be good to have a bit more details in the description > for changes such as this, like a link to the public docs for the extension. I'm sorry, I assumed this information was

[PATCH] D94083: [AArch64] Add +pauth archictecture option, allowing the v8.3a pointer authentication extension.

chill accepted this revision. chill 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/D94083/new/ https://reviews.llvm.org/D94083 ___ cfe

[PATCH] D94081: [AArch64] Add +flagm archictecture option, allowing the v8.4a flag modification extension.

chill accepted this revision. chill 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/D94081/new/ https://reviews.llvm.org/D94081 ___ cf

[PATCH] D91438: [AArch64] Define __ARM_FEATURE_{CRC32,ATOMICS}

chill accepted this revision. chill 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/D91438/new/ https://reviews.llvm.org/D91438 __

[PATCH] D85649: [AArch64] PAC/BTI code generation for LLVM generated functions

This revision was automatically updated to reflect the committed changes. Closed by commit rGa88c722e687e: [AArch64] PAC/BTI code generation for LLVM generated functions (authored by chill). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monore

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

chill requested changes to this revision. chill added a comment. This revision now requires changes to proceed. In D85649 I changed the module flags to be always present and have a zero/non-zero value. That's needed during LTO, if a flag is present in one module

[PATCH] D75044: [AArch64] __builtin_return_address for PAuth.

chill accepted this revision. chill added a comment. In D75044#2292302 , @danielkiss wrote: > @chill ping. Sorry, I thought about committing all PAC/BTI patches together, but there's no reason, is there? So, let's go ahead and commit the two dealing with

[PATCH] D83004: [UpdateCCTestChecks] Include generated functions if asked

chill added a comment. Some tests started failing: http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-ubuntu/builds/9071 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83004/new/ https://reviews.llvm.org/D83004 __

[PATCH] D81930: [AArch64] Add -mmark-bti-property flag.

chill accepted this revision. chill added a comment. This revision is now accepted and ready to land. LGTM. It'd be nice if we could get someone non-Arm to have a look too. though. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81930/new/ https://reviews.llvm.org/D81930

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

chill added a comment. LGTM, as soon as D85649 is accepted (so they stay in sync). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80791/new/ https://reviews.llvm.org/D80791 ___ cfe-commits mailing list cfe-co

[PATCH] D81930: [AArch64] Add -mmark-bti-property flag.

chill added inline comments. Comment at: llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.cpp:106 + + EmitBTIMarking = MarkBTIProperty.getValue(); } No need to the `.getValue()` part. Comment at: llvm/lib/Target/AArch64/MCTargetDesc/AArc

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

chill added a comment. In D85649 I suggested a different version of module flags, which is a bit nicer to use, e.g. one can say just getModuleFlag("sign-return-address-with-bkey") != nullptr instead of a) checking for the flag presence, b) getting its value a

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

chill added a comment. In D80791#2210124 , @danielkiss wrote: >>> it is not useful to have a bti annotated function unless everything else is >>> bti compatible too: it is all or nothing per elf module. >> >> This is false. Some functions in an elf module

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

chill added a comment. In D80791#2209624 , @nsz wrote: > In D80791#2207203 , @chill wrote: > >> I would prefer to avoid the situation where the markings of two otherwise >> identical files were different, >> depend

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

chill added a comment. I would prefer to avoid the situation where the markings of two otherwise identical files were different, depending on how the files were produced, no matter if it was a common or a special case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80791/new/ https://r

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

chill added a comment. In D80791#2206853 , @nsz wrote: > i think that cannot work. > > the implementation is free to inject arbitrary code into > user code so if the user does not tell the implementation > that it wants the entire tu to be bti safe then no

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

chill added a comment. In D80791#2196598 , @nsz wrote: > the assumption is that the intended branch protection is implied via cmdline > flags for the tu and function attributes are only used in source code for > some hack. I don't share this assumption.

[PATCH] D81930: [AArch64] Add -mmark-bti-property flag.

chill requested changes to this revision. chill added inline comments. This revision now requires changes to proceed. Herald added a subscriber: dang. Comment at: llvm/lib/Target/AArch64/AArch64.td:352 +def FeatureEmitNoteBTIProperty : SubtargetFeature<"markbtiproperty", "Mark

[PATCH] D75181: [AArch64] Handle BTI/PAC in case of generated functions.

chill added a comment. In D75181#2193447 , @danielkiss wrote: > I don't see any other alternative option, I'm open to any other idea. My original idea was to pass options to LLVM. I'll come up with a patch in a day or two (if it works) and then we'll see

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

chill added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:5550 +auto &VMContext = CGM.getLLVMContext(); +M->addModuleFlag(llvm::Module::Override, "sign-return-address", + Scope == LangOptions::SignReturnAddressScopeKind::All

[PATCH] D75181: [AArch64] Handle BTI/PAC in case of generated functions.

chill added a comment. This approach looks way too hackish to me with multiple opposing attributes ("sign-return-address" vs. "ignore-sign-return-address") and some convoluted logic to resolve the contradiction. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75181/new/ https://reviews.l

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

chill added a comment. In D80791#2164543 , @danielkiss wrote: >> If any function has the attribute "sign-return-address", then the output note >> section should have PAC bit set. The return address signing is completely >> local >> to the function, and fu

[PATCH] D82949: [Driver][ARM] Disable bf16 when hardware FP support is missing

chill added a comment. Is this patch needed anymore? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82949/new/ https://reviews.llvm.org/D82949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D75044: [AArch64] __builtin_return_address for PAuth.

chill added a comment. In D75044#2186973 , @chill wrote: > Let's postpone this just for a little bit, to settle on an approach to `depth > > 0`. This is with regard to https://reviews.llvm.org/D84502#inline-779900 CHANGES SINCE LAST ACTION https://r

[PATCH] D75044: [AArch64] __builtin_return_address for PAuth.

chill requested changes to this revision. chill added a comment. This revision now requires changes to proceed. Let's postpone this just for a little bit, to settle on an approach to `depth > 0`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75044/new/ https://reviews.llvm.org/D75044

[PATCH] D82948: [Driver][ARM] Disable unsupported features when nofp arch extension is used

chill accepted this revision. chill added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:481 +{"-dotprod", "-fp16fml", "-bf16", "-mve.fp"}); +if (!hasIntegerMVE(Features)) {

[PATCH] D75044: [AArch64] __builtin_return_address for PAuth.

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

[PATCH] D75044: [AArch64] __builtin_return_address for PAuth.

chill added a comment. In D75044#2165496 , @chill wrote: > The issue is that the definition of the instructions `XPAC{D,I}` is > incorrect: it does not mention at all the operand to those insns. Err, they do mention the operand, but only as an input one

[PATCH] D75044: [AArch64] __builtin_return_address for PAuth.

chill added a comment. I'm afraid the patch does not work yet. For example, when the following program void *f() { void g(); g(); return __builtin_return_address(0); } is compiled with ./bin/clang -target aarch64-eabi -march=armv8.3-a -mbranch-protection=pac-ret

[PATCH] D82948: [Driver][ARM] Disable unsupported features when nofp arch extension is used

chill added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:476 +// -mfpu=none, -march=armvX+nofp or -mcpu=X+nofp is *very* similar to +// -mfloat-abi=soft, only that it should not disable MVE-I. Features.insert(Features.end(),

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

chill added a comment. I don't think this behaviour is correct with regard to the specification (AAELF64 2020Q2): > Static linkers processing ELF relocatable objects must set the feature bit in > the output object or image > only if all the input objects have the corresponding feature bit set.

[PATCH] D82948: [Driver][ARM] Disable unsupported features when nofp arch extension is used

chill added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:292-297 + auto checkFPDisabledInArchName = [](const StringRef &ArchName) { +SmallVector Split; +ArchName.split(Split, '+', -1, false); +return llvm::any_of( +Split, [](const Str

[PATCH] D82948: [Driver][ARM] Disable unsupported features when nofp arch extension is used

chill added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:292-297 + auto checkFPDisabledInArchName = [](const StringRef &ArchName) { +SmallVector Split; +ArchName.split(Split, '+', -1, false); +return llvm::any_of( +Split, [](const Str

[PATCH] D82948: [Driver][ARM] Disable unsupported features when nofp arch extension is used

chill added a comment. Needs a regression test. This patch and the dependent patch clash, better with a single patch. Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:288 +static void appendNoFPUnsupportedFeatures(const arm::FloatABI ABI, +

[PATCH] D81837: [ARM][bfloat] Removing lowering of bfloat arguments and returns from Clang's CodeGen

chill added a comment. Pretty straightforward, LGTM. I'd suggest rewording the title (presumably commit message summary) into something like "Do not coerce bfloat arguments and returns to integers", as we're obviously still lowering C and C++ to LLVM LR.§§ Repository: rG LLVM Github Monorepo

cfe-commits@lists.llvm.org

chill added inline comments. Comment at: llvm/test/CodeGen/AArch64/aarch64-bf16-ldst-intrinsics.ll:264 +; Function Attrs: argmemonly nounwind readonly +declare { <8 x bfloat>, <8 x bfloat> } @llvm.aarch64.neon.ld2lane.v8bf16.p0i8(<8 x bfloat>, <8 x bfloat>, i64, i8*) #3 + --

cfe-commits@lists.llvm.org

chill added inline comments. Comment at: llvm/test/CodeGen/AArch64/aarch64-bf16-ldst-intrinsics.ll:264 +; Function Attrs: argmemonly nounwind readonly +declare { <8 x bfloat>, <8 x bfloat> } @llvm.aarch64.neon.ld2lane.v8bf16.p0i8(<8 x bfloat>, <8 x bfloat>, i64, i8*) #3 + --

cfe-commits@lists.llvm.org

chill added inline comments. Comment at: llvm/test/CodeGen/AArch64/aarch64-bf16-ldst-intrinsics.ll:264 +; Function Attrs: argmemonly nounwind readonly +declare { <8 x bfloat>, <8 x bfloat> } @llvm.aarch64.neon.ld2lane.v8bf16.p0i8(<8 x bfloat>, <8 x bfloat>, i64, i8*) #3 + --

[PATCH] D79693: [test][ARM][CMSE] Use clang_cc1 in arm_cmse.h tests

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

[PATCH] D79693: [test][ARM][CMSE] Use -ffreestanding for arm_cmse.h tests

chill added a comment. I see. I can also count (`grep -rn '#include.*https://reviews.llvm.org/D79693/new/ https://reviews.llvm.org/D79693 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D79693: [test][ARM][CMSE] Use -ffreestanding for arm_cmse.h tests

chill added a comment. I'm sorry, I don't understand the issue. Certainly it's the compiler (driver) responsibility to setup include paths according to the selected target. How do you trigger a problem? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D78129: Add Marvell ThunderX3T110 support

chill added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:849-857 +// These pointer authentication instructions require armv8.3a +let Predicates = [HasV8_3a, HasPA] in { let Uses = [LR], Defs = [LR] in { def PACIAZ : SystemNoOperands<0b000, "hint\

[PATCH] D76369: [CMSE] Clear padding bits of struct/unions/fp16 passed by value

This revision was automatically updated to reflect the committed changes. Closed by commit rG102b4105e3fd: [CMSE] Clear padding bits of struct/unions/fp16 passed by value (authored by chill). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monor

[PATCH] D77270: Fix the check for regparm in FunctionType::ExtInfo

This revision was automatically updated to reflect the committed changes. Closed by commit rG334ac8105401: Fix the check for regparm in FunctionType::ExtInfo (authored by chill). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D75181: [AArch64] Handle BTI/PAC in case of generated functions.

chill added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:5149-5152 + if (BPI.BranchTargetEnforcement) +Fn->addFnAttr("branch-target-enforcement", "true"); + else +Fn->addFnAttr("branch-target-enforcement", "false"); -

[PATCH] D75044: [AArch64] __builtin_extract_return_addr for PAuth.

chill added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:5151 + return Address; +} +llvm::Function *F = Can drop the extra braces here. Comment at: clang/lib/CodeGen/TargetInfo.cpp:5156 +llvm::CallInst *Call =

[PATCH] D75044: [AArch64] __builtin_extract_return_addr for PAuth.

chill added a comment. In D75044#1966997 , @chill wrote: > Needs a test in `clang/test` that `__builtin_extract_return_address` is > translated to `llvm.extractreturnaddress`. Nevermind, I'm blind. CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D75044: [AArch64] __builtin_extract_return_addr for PAuth.

chill added a comment. Needs a test in `clang/test` that `__builtin_extract_return_address` is translated to `llvm.extractreturnaddress`. What if LLVM IR contains a call to `llvm.extractreturnaddress`, but the target is not AArch64? Comment at: llvm/include/llvm/CodeGen/ISDO

[PATCH] D71129: [ARM][CMSE] Implement CMSE attributes

chill marked 4 inline comments as done. chill added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:1882 + if (FI.isCmseNSCall()) +FuncAttrs.addAttribute("cmse_nonsecure_call"); + snidertm wrote: > Just curious … Does the LLVM backend have a way to

[PATCH] D71129: [ARM][CMSE] Implement CMSE attributes

chill added a comment. In D71129#1956137 , @snidertm wrote: > Have you already committed support for CMSE attributes to the LLVM backend? > Or is that on the way? The last two CMSE patches are under review: https://reviews.llvm.org/D76518 Repository:

[PATCH] D71129: [ARM][CMSE] Implement CMSE attributes

chill marked 2 inline comments as done. chill added inline comments. Comment at: clang/include/clang/AST/Type.h:3622 bool getNoCfCheck() const { return Bits & NoCfCheckMask; } bool getHasRegParm() const { return (Bits >> RegParmOffset) != 0; } snidert

[PATCH] D71129: [ARM][CMSE] Implement CMSE attributes

chill marked 3 inline comments as done. chill added inline comments. Comment at: clang/include/clang/AST/Type.h:3588 + NoCallerSavedRegsMask | NoCfCheckMask | CmseNSCallMask), RegParmOffset = 8 }; // Assumed to be the last field s

[PATCH] D77134: [clang][AARCH64] Add __ARM_FEATURE_{PAC, BTI}_DEFAULT defines

chill accepted this revision. chill added a comment. This revision is now accepted and ready to land. LGTM, conditional on the dependent patch. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77134/new/ https://reviews.llvm.org/D77134

[PATCH] D77131: [clang] Move branch-protection from CodeGenOptions to LangOptions

chill added a reviewer: efriedma. chill added a subscriber: efriedma. chill added a comment. Following @efriedma comment here http://lists.llvm.org/pipermail/cfe-dev/2020-March/065017.html LGTM. Comment at: clang/include/clang/Basic/TargetInfo.h:18 #include "clang/Basic/Addr

[PATCH] D75181: [AArch64] Handle BTI/PAC in case of generated functions.

chill added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:1828 + if (CodeGenOpts.BranchTargetEnforcement) { +FuncAttrs.addAttribute("branch-target-enforcement", "true"); + } danielkiss wrote: > chill wrote: > > I would really prefer to not set v

[PATCH] D71129: [ARM][CMSE] Implement CMSE attributes

This revision was automatically updated to reflect the committed changes. Closed by commit rG080d046c91d2: [ARM][CMSE] Implement CMSE attributes (authored by chill). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D75109: Apply function attributes through array declarators

This revision was automatically updated to reflect the committed changes. Closed by commit rG6081ccf4a3b6: Apply function attributes through array declarators (authored by chill). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D75181: [AArch64] Handle BTI/PAC in case of generated functions.

chill added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp:200 +if (!F.hasFnAttribute("branch-target-enforcement")) + return false; +Attribute A = F.getFnAttribute("branch-target-enforcement"); chill wrote: > T

[PATCH] D75181: [AArch64] Handle BTI/PAC in case of generated functions.

chill added a comment. That said, my comments are not of the "over my dead body" kind ;) Comment at: clang/lib/CodeGen/CGCall.cpp:1828 + if (CodeGenOpts.BranchTargetEnforcement) { +FuncAttrs.addAttribute("branch-target-enforcement", "true"); + } I would r

[PATCH] D74966: [PATCH] [ARM] Add Cortex-M55 Support for clang and llvm

chill accepted this revision. chill added a comment. This revision is now accepted and ready to land. LGTM. Please, wait a couple of days before committing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74966/new/ https://reviews.llvm.org/D74966 __

[PATCH] D72633: [ARM][MVE] Fix a corner case of checking for MVE-I with -mfpu=none

This revision was automatically updated to reflect the committed changes. Closed by commit rGda3f2b414ace: [ARM][MVE] Fix a corner case of checking for MVE-I with -mfpu=none (authored by chill). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: http

  1   2   >