[llvm-branch-commits] [clang] [clang] Define ptrauth_sign_constant builtin. (PR #93904)
efriedma-quic wrote: Why do we want a separate builtin, as opposed to just constant-folding calls to __builtin_ptrauth_sign? https://github.com/llvm/llvm-project/pull/93904 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [clang] Define ptrauth_sign_constant builtin. (PR #93904)
@@ -354,6 +354,23 @@ Given that ``signedPointer`` matches the layout for signed pointers signed with the given key, extract the raw pointer from it. This operation does not trap and cannot fail, even if the pointer is not validly signed. +``ptrauth_sign_constant`` +^ + +.. code-block:: c + + ptrauth_sign_constant(pointer, key, discriminator) + +Return a signed pointer for a constant address in a manner which guarantees +a non-attackable sequence. + +``pointer`` must be a constant expression of pointer type which evaluates to +a non-null pointer. The result will have the same type as ``discriminator``. + +Calls to this are constant expressions if the discriminator is a null-pointer +constant expression or an integer constant expression. Implementations may +allow other pointer expressions as well. efriedma-quic wrote: This seems to imply that if you don't need a constant expression, the discriminator can be a variable. This doesn't seem to match the implementation, though: it requires a constant discriminator in a specific form. https://github.com/llvm/llvm-project/pull/93904 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Hashing] Use a non-deterministic seed (PR #96282)
efriedma-quic wrote: We restricted reverse-iteration when we added it just to save time when we were enabling it: we wanted to prioritize issues that were actually likely to cause non-determinism (as opposed to relying on the hash algorithm, which is annoying but not actually non-deterministic). If you're willing to fix all the resulting breakage, it should be fine to apply it more widely. - I'm a little concerned that doing this in release builds is going to lead to weird bug reports. Especially given the current approach for getting randomness: ASLR isn't really that random, particularly on Windows, so the probability of getting a particular seed isn't uniform. https://github.com/llvm/llvm-project/pull/96282 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [Hashing] Use a non-deterministic seed if LLVM_ENABLE_ABI_BREAKING_CHECKS (PR #96282)
https://github.com/efriedma-quic edited https://github.com/llvm/llvm-project/pull/96282 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [Hashing] Use a non-deterministic seed if LLVM_ENABLE_ABI_BREAKING_CHECKS (PR #96282)
@@ -322,24 +306,20 @@ struct hash_state { } }; - -/// A global, fixed seed-override variable. -/// -/// This variable can be set using the \see llvm::set_fixed_execution_seed -/// function. See that function for details. Do not, under any circumstances, -/// set or read this variable. -extern uint64_t fixed_seed_override; - +/// In LLVM_ENABLE_ABI_BREAKING_CHECKS builds, the seed is non-deterministic +/// (address of a variable) to prevent having users depend on the particular +/// hash values. On platforms without ASLR, this is still likely +/// non-deterministic per build. inline uint64_t get_execution_seed() { - // FIXME: This needs to be a per-execution seed. This is just a placeholder - // implementation. Switching to a per-execution seed is likely to flush out - // instability bugs and so will happen as its own commit. - // - // However, if there is a fixed seed override set the first time this is - // called, return that instead of the per-execution seed. - const uint64_t seed_prime = 0xff51afd7ed558ccdULL; - static uint64_t seed = fixed_seed_override ? fixed_seed_override : seed_prime; - return seed; + // Work around x86-64 negative offset folding for old Clang -fno-pic + // https://reviews.llvm.org/D93931 +#if LLVM_ENABLE_ABI_BREAKING_CHECKS && \ +(!defined(__clang__) || __clang_major__ > 11) efriedma-quic wrote: Is it an ABI problem that this ifdef exists? I mean, LLVM libraries built with clang<11 can't be used by programs built with clang>11. With LLVM_ENABLE_ABI_BREAKING_CHECKS, I guess it's unlikely to cause issues, though. (I guess you could use an empty inline asm as a workaround if you wanted to.) https://github.com/llvm/llvm-project/pull/96282 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [Hashing] Use a non-deterministic seed if LLVM_ENABLE_ABI_BREAKING_CHECKS (PR #96282)
https://github.com/efriedma-quic commented: I think I'm happier restricting the non-determinism to +Asserts for now, at least as an incremental step. > Due to Avalanche effects, even a few ASLR bits are sufficient to cover many > different scenarios and expose latent bugs. On Windows specifically, I'm less concerned about the total number of bits, and more concerned that ASLR isn't randomized for each run of an executable. https://github.com/llvm/llvm-project/pull/96282 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/19.x: [clang][ARM64EC] Add support for hybrid_patchable attribute. (#99478) (PR #100873)
efriedma-quic wrote: LGTM https://github.com/llvm/llvm-project/pull/100873 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] release/19.x: [CodeGen][ARM64EC] Use alias symbol for exporting hybrid_patchable functions. (#100872) (PR #101178)
efriedma-quic wrote: LGTM https://github.com/llvm/llvm-project/pull/101178 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [PowerPC][GlobalMerge] Reduce TOC usage by merging internal and private global data (PR #101224)
@@ -28,6 +28,10 @@ struct GlobalMergeOptions { bool MergeConst = false; /// Whether we should merge global variables that have external linkage. bool MergeExternal = true; + /// Whether we should merge global variables that have private linkage. + bool MergePrivateGlobals = false; efriedma-quic wrote: MergePrivateGlobals is unused? https://github.com/llvm/llvm-project/pull/101224 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] release/19.x: backport PR96483 (PR #102491)
https://github.com/efriedma-quic approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/102491 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] release/19.x: [BinaryFormat] Disable MachOTest.UnalignedLC on SPARC (#100086) (PR #102103)
https://github.com/efriedma-quic approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/102103 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] release/19.x: [CodeGen][ARM64EC] Define hybrid_patchable EXP thunk symbol as a function. (#102898) (PR #103048)
https://github.com/efriedma-quic approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/103048 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] AArch64: Use consistent atomicrmw expansion for FP operations (PR #103702)
efriedma-quic wrote: Please also fix the comment before AArch64TargetLowering::shouldExpandAtomicRMWInIR . (The comment is wrong; a floating-point "exception" isn't an exception in the sense that it would cancel an exclusive reservation.) https://github.com/llvm/llvm-project/pull/103702 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] AArch64: Use consistent atomicrmw expansion for FP operations (PR #103702)
efriedma-quic wrote: Just thought of this, but... we can't do this in the case where we do a libcall. Any load or store between the load exclusive and the store exclusive could break the reservation. (It normally won't, but it can in weird cases where the atomic variable is on the stack.) So we have to use cmpxchg lowering in those cases (and then expand the cmpxchg to ll/sc). The cases we can do inline should be fine, though. https://github.com/llvm/llvm-project/pull/103702 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] Backport ARM64EC variadic args fixes to LLVM 18 (PR #81800)
efriedma-quic wrote: I was sort of waiting until the discussion on #80994 resolves... we might end up reverting parts of #80595 . I guess it won't do any harm to land as-is, though. https://github.com/llvm/llvm-project/pull/81800 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] Backport ARM64EC variadic args fixes to LLVM 18 (PR #81800)
https://github.com/efriedma-quic commented: Looks fine, but I think merging might need to wait for a point release? CC @tstellar . https://github.com/llvm/llvm-project/pull/81800 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] release/18.x: [ARM] Switch to LiveRegUnits to fix r7 register allocation bug (PR #84475)
https://github.com/efriedma-quic requested changes to this pull request. This is, as far as I can tell, not a miscompile; probably not worth taking on the 18.x branch. (Also, it's usually not a good idea to open a PR for a cherry-pick before the original patch is merged.) https://github.com/llvm/llvm-project/pull/84475 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [ObjC] Check entire chain of superclasses to determine class layout (PR #84093)
https://github.com/efriedma-quic requested changes to this pull request. We usually only take bugfixes on release branches (miscompiles/crashes/etc.). https://github.com/llvm/llvm-project/pull/84093 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] Backport ARM64EC variadic args fixes to LLVM 18 (PR #81800)
https://github.com/efriedma-quic approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/81800 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [lld] [llvm] Backport fixes for ARM64EC import libraries (PR #84590)
efriedma-quic wrote: This seems like a pretty big change to backport... how useful is it in practice? I was under the impression that arm64ec lld support is still immature... and if you're using the MSVC linker, you might as well use the MSVC lib/dlltool. https://github.com/llvm/llvm-project/pull/84590 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [lld] [llvm] Backport fixes for ARM64EC import libraries (PR #84590)
efriedma-quic wrote: So your use-case is basically equivalent to using llvm-dlltool, except not using the text parser? If this is actually enough to make Rust targets usable, then I guess we could consider it, but the fixes aren't structured in a way to make it obvious this won't impact non-ARM64EC targets. https://github.com/llvm/llvm-project/pull/84590 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x [X86_64] fix SSE type error in vaarg (PR #86698)
efriedma-quic wrote: Is there some reason you think we should take this specific patch, out of all the x86 ABI fixes going in recently? It isn't a regression, as far as I know. https://github.com/llvm/llvm-project/pull/86698 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang][CallGraphSection] Add type id metadata to indirect call and targets (PR #87573)
@@ -93,9 +93,17 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorCall( *this, MD, This, ImplicitParam, ImplicitParamTy, CE, Args, RtlArgs); auto &FnInfo = CGM.getTypes().arrangeCXXMethodCall( Args, FPT, CallInfo.ReqArgs, CallInfo.PrefixSize); - return EmitCall(FnInfo, Callee, ReturnValue, Args, nullptr, + llvm::CallBase *CallOrInvoke = nullptr; + auto Call = EmitCall(FnInfo, Callee, ReturnValue, Args, &CallOrInvoke, CE && CE == MustTailCall, CE ? CE->getExprLoc() : SourceLocation()); + + // Set type identifier metadata of indirect calls for call graph section. + if (CGM.getCodeGenOpts().CallGraphSection && CallOrInvoke && + CallOrInvoke->isIndirectCall()) +CGM.CreateFunctionTypeMetadataForIcall(MD->getType(), CallOrInvoke); efriedma-quic wrote: This seems like it's scattering calls to CreateFunctionTypeMetadataForIcall across a lot of different places; code like this is hard to maintain. Is there some reason we can't just do it in EmitCall() itself, or something like that? https://github.com/llvm/llvm-project/pull/87573 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] Backport: Prepend all library intrinsics with `#` when building for Arm64EC (PR #88016)
efriedma-quic wrote: LGTM. (This only affects Arm64EC, so it's very safe to backport.) https://github.com/llvm/llvm-project/pull/88016 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x [X86_64] fix SSE type error in vaarg (PR #86698)
efriedma-quic wrote: Right, the policy doesn't say we can only take regression fixes. We just need to weight the impact vs. the risk. Looking at the latest conversation on the bug report this case is pretty clearly still broken. It's improved in the sense that after the va_arg of the struct, subsequent va_arg calls produce the right value. But the va_arg iteslf doesn't produce the right value (probably we aren't copying the struct correctly). So that would be a regression for some cases. Given that, we probably don't want to pull this into 18.x as-is. Also, given that we're making other fixes to the surrounding code, pulling any one fix into 18.x seems risky to me. And probably low-impact, given the testcases appear to be generated by a fuzzer. https://github.com/llvm/llvm-project/pull/86698 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [Clang] Handle structs with inner structs and no fields (#89126) (PR #89456)
efriedma-quic wrote: LGTM This only impacts code using dynamic object sizes, which... I'm not sure how widely it's actually used outside the Linux kernel. Implementation-wise, should be pretty safe. There's some minor risk because the revised recursion visits RecordDecls it wouldn't look into before, but that seems unlikely to cause a practical issue. https://github.com/llvm/llvm-project/pull/89456 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [release/18.x][COFF][Aarch64] Add _InterlockedAdd64 intrinsic (#81849) (PR #89951)
efriedma-quic wrote: I think BuiltinsAArch64.def is part of clang's ABI, so changing it violates the backport rules. Otherwise, I'd be inclined to accept; it's kind of late to request, but it's low risk. https://github.com/llvm/llvm-project/pull/89951 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [Clang] Handle structs with inner structs and no fields (#89126) (PR #90133)
efriedma-quic wrote: Looks fine. https://github.com/llvm/llvm-project/pull/90133 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [clang][builtin] Implement __builtin_allow_runtime_check (PR #87568)
efriedma-quic wrote: I think it's worth re-posting the builtin as a separate RFC on Discourse, since the original RFC hadn't settled on the exact design for the clang builtin you're using here. Code changes look fine. https://github.com/llvm/llvm-project/pull/87568 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang codegen] Fix MS ABI detection of user-provided constructors. (#90151) (PR #90639)
efriedma-quic wrote: Proposing for backport because this is high-impact for anyone using Qt on Arm64 Windows. https://github.com/llvm/llvm-project/pull/90639 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] Backport "riscv-isa" module metadata to 18.x (PR #91514)
efriedma-quic wrote: Can you briefly summarize why this is important to backport? At first glance, this is only relevant for LTO with mixed architecture specifications, which... I can see someone might want it, I guess, but it seems pretty easy to work around not having it. https://github.com/llvm/llvm-project/pull/91514 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] Backport "riscv-isa" module metadata to 18.x (PR #91514)
efriedma-quic wrote: If LTO was completely broken, this seems worth taking. And the changes look safe. LGTM. https://github.com/llvm/llvm-project/pull/91514 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] release/18.x: [GlobalOpt] Don't replace aliasee with alias that has weak linkage (#91483) (PR #92468)
https://github.com/efriedma-quic approved this pull request. LGTM This only affects optimizations on weak aliases, and the logic is very simple: just don't optimize them. As noted on the original pull request, this also affects some cases which might be safe to optimize (a weak alias where the aliasee is an internal symbol with no other references). But "optimizing" those cases doesn't really have any useful effect, anyway: it doesn't unblock any additional optimizations, and the resulting ELF is basically identical. https://github.com/llvm/llvm-project/pull/92468 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [release/18.x] Backport fixes for ARM64EC thunk generation (PR #92580)
https://github.com/efriedma-quic approved this pull request. LGTM This only affects Arm64EC targets, the fixes are relatively small, and this affects correctness of generated thunks. https://github.com/llvm/llvm-project/pull/92580 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [LivePhysRegs] Add callee-saved regs from MFI in addLiveOutsNoPristines. (PR #73553)
efriedma-quic wrote: > After PEI the liveness of LR needs to be accurately reflected and tail calls > could (should?) always "use" LR. That would either prevent outlining or cause > the outliner to preserve LR across introduced calls. I'll elaborate on this a bit. I think long-term, the way we want to model this is that pre-PEI, LR is treated as if it were a callee-save register. (Nothing outside the prologue/epilogue should interact with the return address directly except for a few special cases like __builtin_return_address.) Then PEI marks the LR use on the relevant "return" instructions, and post-PEI it's not callee-save anymore. I think that correctly models the underlying weirdness: at a machine level, the return address is basically just an argument to the function, but it's special to PEI because of the interaction with the frame layout/exception handling/pop instructions/etc. Stuff before PEI doesn't need to be aware it's an argument, and stuff after PEI can just treat it as a normal register. > On the caller side, the call instruction clobbers LR, so it can't really be > considered live-out Correct, it's not really live-out. --- Short-term, can we make this fix a bit more targeted? The point of the "isRestored" bit is that it's supposed to avoid marking liveness when the function ends in a "pop pc". The relevant code is ARMFrameLowering::emitPopInst: it calls `Info.setRestored(false);` to do precisely this. But it's triggering in a case where it shouldn't. I think the problem is that the code isn't accounting for the possibility that there are other paths that return from the function. https://github.com/llvm/llvm-project/pull/73553 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [LivePhysRegs] Add callee-saved regs from MFI in addLiveOutsNoPristines. (PR #73553)
efriedma-quic wrote: Just looked at https://gist.github.com/fhahn/67937125b64440a8a414909c4a1b7973 ; that seems roughly appropriate. It's a little ugly to set the bit to false, then set it back to true, though; I'd rather just explicitly check whether all return instructions are LDMIA_RET/t2LDMIA_RET/tPOP_RET. https://github.com/llvm/llvm-project/pull/73553 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] AArch64: Use consistent atomicrmw expansion for FP operations (PR #103702)
@@ -27056,21 +27056,35 @@ AArch64TargetLowering::shouldExpandAtomicLoadInIR(LoadInst *LI) const { : AtomicExpansionKind::LLSC; } +// Return true if the atomic operation expansion will lower to use a library +// call, and is thus ineligible to use an LLSC expansion. +static bool rmwOpMayLowerToLibcall(const AtomicRMWInst *RMW) { + if (!RMW->isFloatingPointOperation()) +return false; + switch (RMW->getType()->getScalarType()->getTypeID()) { + case Type::FloatTyID: + case Type::DoubleTyID: + case Type::HalfTyID: + case Type::BFloatTyID: +return false; efriedma-quic wrote: We need to check if we're in softfp mode (`Subtarget->hasFPARMv8()`). https://github.com/llvm/llvm-project/pull/103702 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] AArch64: Use consistent atomicrmw expansion for FP operations (PR #103702)
@@ -27056,21 +27056,35 @@ AArch64TargetLowering::shouldExpandAtomicLoadInIR(LoadInst *LI) const { : AtomicExpansionKind::LLSC; } +// Return true if the atomic operation expansion will lower to use a library +// call, and is thus ineligible to use an LLSC expansion. +static bool rmwOpMayLowerToLibcall(const AtomicRMWInst *RMW) { + if (!RMW->isFloatingPointOperation()) +return false; + switch (RMW->getType()->getScalarType()->getTypeID()) { + case Type::FloatTyID: + case Type::DoubleTyID: + case Type::HalfTyID: + case Type::BFloatTyID: +return false; efriedma-quic wrote: aarch64 has fp enabled by default; you need something like `-mattr=-fp-armv8` to test. https://github.com/llvm/llvm-project/pull/103702 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] AArch64: Use consistent atomicrmw expansion for FP operations (PR #103702)
@@ -27056,21 +27056,35 @@ AArch64TargetLowering::shouldExpandAtomicLoadInIR(LoadInst *LI) const { : AtomicExpansionKind::LLSC; } +// Return true if the atomic operation expansion will lower to use a library +// call, and is thus ineligible to use an LLSC expansion. +static bool rmwOpMayLowerToLibcall(const AtomicRMWInst *RMW) { + if (!RMW->isFloatingPointOperation()) +return false; + switch (RMW->getType()->getScalarType()->getTypeID()) { + case Type::FloatTyID: + case Type::DoubleTyID: + case Type::HalfTyID: + case Type::BFloatTyID: +return false; efriedma-quic wrote: (I'm confident Subtarget->hasFPARMv8() is the right check; it's the same check used to enable the registers.) https://github.com/llvm/llvm-project/pull/103702 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] AArch64: Use consistent atomicrmw expansion for FP operations (PR #103702)
https://github.com/efriedma-quic approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/103702 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/19.x: [Clang][CodeGen] Don't emit assumptions if current block is unreachable. (#106936) (PR #107183)
https://github.com/efriedma-quic approved this pull request. LGTM. Fixes a crash on valid, and the fix is very unlikely to have unexpected effects. https://github.com/llvm/llvm-project/pull/107183 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] 4a51298 - [AArch64][SVE] Implement SPLAT_VECTOR for i1 vectors.
Author: Eli Friedman Date: 2019-12-09T15:04:40-08:00 New Revision: 4a51298c13005be05e100f0ef46dbac47623bcd6 URL: https://github.com/llvm/llvm-project/commit/4a51298c13005be05e100f0ef46dbac47623bcd6 DIFF: https://github.com/llvm/llvm-project/commit/4a51298c13005be05e100f0ef46dbac47623bcd6.diff LOG: [AArch64][SVE] Implement SPLAT_VECTOR for i1 vectors. The generated sequence with whilelo is unintuitive, but it's the best I could come up with given the limited number of SVE instructions that interact with scalar registers. The other sequence I was considering was something like dup+cmpne, but an extra scalar instruction seems better than an extra vector instruction. Differential Revision: https://reviews.llvm.org/D71160 Added: Modified: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp llvm/test/CodeGen/AArch64/sve-vector-splat.ll Removed: diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp index f32f03741221..b42496abecb6 100644 --- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp +++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp @@ -825,7 +825,7 @@ AArch64TargetLowering::AArch64TargetLowering(const TargetMachine &TM, // splat of 0 or undef) once vector selects supported in SVE codegen. See // D68877 for more details. for (MVT VT : MVT::integer_scalable_vector_valuetypes()) { - if (isTypeLegal(VT) && VT.getVectorElementType() != MVT::i1) + if (isTypeLegal(VT)) setOperationAction(ISD::SPLAT_VECTOR, VT, Custom); } setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::i8, Custom); @@ -7135,26 +7135,31 @@ SDValue AArch64TargetLowering::LowerSPLAT_VECTOR(SDValue Op, switch (ElemVT.getSimpleVT().SimpleTy) { case MVT::i8: case MVT::i16: + case MVT::i32: SplatVal = DAG.getAnyExtOrTrunc(SplatVal, dl, MVT::i32); -break; +return DAG.getNode(AArch64ISD::DUP, dl, VT, SplatVal); case MVT::i64: SplatVal = DAG.getAnyExtOrTrunc(SplatVal, dl, MVT::i64); -break; - case MVT::i32: -// Fine as is -break; - // TODO: we can support splats of i1s and float types, but haven't added - // patterns yet. - case MVT::i1: +return DAG.getNode(AArch64ISD::DUP, dl, VT, SplatVal); + case MVT::i1: { +// The general case of i1. There isn't any natural way to do this, +// so we use some trickery with whilelo. +// TODO: Add special cases for splat of constant true/false. +SplatVal = DAG.getAnyExtOrTrunc(SplatVal, dl, MVT::i64); +SplatVal = DAG.getNode(ISD::SIGN_EXTEND_INREG, dl, MVT::i64, SplatVal, + DAG.getValueType(MVT::i1)); +SDValue ID = DAG.getTargetConstant(Intrinsic::aarch64_sve_whilelo, dl, + MVT::i64); +return DAG.getNode(ISD::INTRINSIC_WO_CHAIN, dl, VT, ID, + DAG.getConstant(0, dl, MVT::i64), SplatVal); + } + // TODO: we can support float types, but haven't added patterns yet. case MVT::f16: case MVT::f32: case MVT::f64: default: -llvm_unreachable("Unsupported SPLAT_VECTOR input operand type"); -break; +report_fatal_error("Unsupported SPLAT_VECTOR input operand type"); } - - return DAG.getNode(AArch64ISD::DUP, dl, VT, SplatVal); } static bool resolveBuildVector(BuildVectorSDNode *BVN, APInt &CnstBits, diff --git a/llvm/test/CodeGen/AArch64/sve-vector-splat.ll b/llvm/test/CodeGen/AArch64/sve-vector-splat.ll index b3f6cb4b24a1..086241c4e0a7 100644 --- a/llvm/test/CodeGen/AArch64/sve-vector-splat.ll +++ b/llvm/test/CodeGen/AArch64/sve-vector-splat.ll @@ -93,3 +93,43 @@ define @sve_splat_2xi32(i32 %val) { %splat = shufflevector %ins, undef, zeroinitializer ret %splat } + +define @sve_splat_2xi1(i1 %val) { +; CHECK-LABEL: @sve_splat_2xi1 +; CHECK: sbfx x8, x0, #0, #1 +; CHECK-NEXT: whilelo p0.d, xzr, x8 +; CHECK-NEXT: ret + %ins = insertelement undef, i1 %val, i32 0 + %splat = shufflevector %ins, undef, zeroinitializer + ret %splat +} + +define @sve_splat_4xi1(i1 %val) { +; CHECK-LABEL: @sve_splat_4xi1 +; CHECK: sbfx x8, x0, #0, #1 +; CHECK-NEXT: whilelo p0.s, xzr, x8 +; CHECK-NEXT: ret + %ins = insertelement undef, i1 %val, i32 0 + %splat = shufflevector %ins, undef, zeroinitializer + ret %splat +} + +define @sve_splat_8xi1(i1 %val) { +; CHECK-LABEL: @sve_splat_8xi1 +; CHECK: sbfx x8, x0, #0, #1 +; CHECK-NEXT: whilelo p0.h, xzr, x8 +; CHECK-NEXT: ret + %ins = insertelement undef, i1 %val, i32 0 + %splat = shufflevector %ins, undef, zeroinitializer + ret %splat +} + +define @sve_splat_16xi1(i1 %val) { +; CHECK-LABEL: @sve_splat_16xi1 +; CHECK: sbfx x8, x0, #0, #1 +; CHECK-NEXT: whilelo p0.b, xzr, x8 +; CHECK-NEXT: ret + %ins = insertelement undef, i1 %val, i32 0 + %splat = shufflevector %ins, undef, zeroinitializer + ret %splat +} __
[llvm-branch-commits] [clang] release/19.x: [Headers] [ARM64EC] Fix extra tokens inside intrin0.h preprocessor directive (#112066) (PR #112258)
https://github.com/efriedma-quic approved this pull request. LGTM. Obvious typo, obvious fix, very low chance of impacting non-arm64ec targets. https://github.com/llvm/llvm-project/pull/112258 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] AtomicExpand: Copy metadata from atomicrmw to cmpxchg (PR #109409)
https://github.com/efriedma-quic approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/109409 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [Clang] Fix 'counted_by' for nested struct pointers (#110497) (PR #111445)
https://github.com/efriedma-quic approved this pull request. LGTM This should be safe to merge: it only affects usage of the new counted_by attribute, and this fixes a significant bug blocking usage of that feature. https://github.com/llvm/llvm-project/pull/111445 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] AtomicExpand: Copy metadata from atomicrmw to cmpxchg (PR #109409)
efriedma-quic wrote: Did you address https://github.com/llvm/llvm-project/pull/109409#pullrequestreview-2332197449 ? https://github.com/llvm/llvm-project/pull/109409 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/19.x: [clang-repl] [codegen] Reduce the state in TBAA. NFC for static compilation. (#98138) (PR #111953)
https://github.com/efriedma-quic approved this pull request. LGTM I'm a little concerned about messing with datastructures in headers, but I think, since the headers in question aren't exposed in clang/include, this doesn't have any visible ABI effect. Otherwise this is a pretty safe refactoring. https://github.com/llvm/llvm-project/pull/111953 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [x86][Windows] Fix chromium build break (PR #111218)
https://github.com/efriedma-quic approved this pull request. LGTM. Fixes a regression, and should be low-risk (the fix only affects the exact functions on the exact target with the issue). https://github.com/llvm/llvm-project/pull/111218 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [x86][Windows] Fix chromium build break (PR #111218)
https://github.com/efriedma-quic edited https://github.com/llvm/llvm-project/pull/111218 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [x86][Windows] Fix chromium build break (PR #111218)
@@ -177,6 +177,107 @@ define float @tan(float %x) #0 { ret float %result } +define float @acos(float %x) #0 { efriedma-quic wrote: Okay that's fine. https://github.com/llvm/llvm-project/pull/111218 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [x86][Windows] Fix chromium build break (PR #111218)
https://github.com/efriedma-quic milestoned https://github.com/llvm/llvm-project/pull/111218 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] AtomicExpand: Copy metadata from atomicrmw to cmpxchg (PR #109409)
efriedma-quic wrote: > (which for some reason is not emitted as an atomic). In terms of the generated code, I think we're fine; we don't rely on the load producing a value that's consistent with atomic ordering, I think. That said, it probably should be atomic, because strictly speaking a race is UB if we use a non-atomic load. https://github.com/llvm/llvm-project/pull/109409 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] AMDGPU: Custom expand flat cmpxchg which may access private (PR #109410)
@@ -25,20 +25,29 @@ bool llvm::lowerAtomicCmpXchgInst(AtomicCmpXchgInst *CXI) { Value *Cmp = CXI->getCompareOperand(); Value *Val = CXI->getNewValOperand(); - LoadInst *Orig = - Builder.CreateAlignedLoad(Val->getType(), Ptr, CXI->getAlign()); - Value *Equal = Builder.CreateICmpEQ(Orig, Cmp); - Value *Res = Builder.CreateSelect(Equal, Val, Orig); - Builder.CreateAlignedStore(Res, Ptr, CXI->getAlign()); + auto [Orig, Equal] = + buildAtomicCmpXchgValue(Builder, Ptr, Cmp, Val, CXI->getAlign()); - Res = Builder.CreateInsertValue(PoisonValue::get(CXI->getType()), Orig, 0); + Value *Res = + Builder.CreateInsertValue(PoisonValue::get(CXI->getType()), Orig, 0); Res = Builder.CreateInsertValue(Res, Equal, 1); CXI->replaceAllUsesWith(Res); CXI->eraseFromParent(); return true; } +std::pair +llvm::buildAtomicCmpXchgValue(IRBuilderBase &Builder, Value *Ptr, Value *Cmp, efriedma-quic wrote: Putting "atomic" in the name here is a little strange; this isn't atomic. https://github.com/llvm/llvm-project/pull/109410 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] AtomicExpand: Copy metadata from atomicrmw to cmpxchg (PR #109409)
https://github.com/efriedma-quic commented: I'm not sure we can assume all metadata which applies to an atomicrmw also makes sense on the generated cmpxchg. I mean, most metadata probably does, but say we start allowing `!align` metadata on atomicrmw... https://github.com/llvm/llvm-project/pull/109409 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] release/19.x: [llvm][aarch64] Fix Arm64EC name mangling algorithm (#115567) (PR #116273)
https://github.com/efriedma-quic approved this pull request. LGTM I think this is safe for the branch: Functionally, this should only affect Arm64EC targets, and this is important for those targets. The header modifications only add functions; nothing is removed or modified, so there shouldn't be ABI compatibility issues. https://github.com/llvm/llvm-project/pull/116273 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/20.x: [clang] Support member function poiners in Decl::getFunctionType() (#125077) (PR #125956)
https://github.com/efriedma-quic approved this pull request. LGTM. This doesn't seem like a high priority to backport, but I guess it's safe enough for this point in the process. https://github.com/llvm/llvm-project/pull/125956 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [libclang] Replace createRef with createDup (PR #126683)
https://github.com/efriedma-quic approved this pull request. LGTM. Looking at the code, none of these codepaths should be hot, so I don't expect any significant performance difference. https://github.com/llvm/llvm-project/pull/126683 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/20.x: [clang][SME] Account for C++ lambdas in SME builtin diagnostics (#124750) (PR #125049)
https://github.com/efriedma-quic approved this pull request. LGTM. Safe bugfix. https://github.com/llvm/llvm-project/pull/125049 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [IR] Add FPOperation intrinsic property (PR #122313)
efriedma-quic wrote: Should this be part of the "memory" attribute, instead of an independent thing? In my head, the model I have is the following: the current fp state is a bit of thread-local state, and transforms that use generic reasoning about memory reads/writes should be able to conservatively handle fp state without knowing anything about it. Then we have some additional markings that allow fp-aware transforms to be more precise: they can see if the operation reads the status flags, or whether the rounding mode is known. I don't think attributes specific to intrinsics make sense in this context; anything that makes sense to assert about an intrinsic call, equally makes sense to assert about a non-intrinsic call. https://github.com/llvm/llvm-project/pull/122313 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [SDAG] Introduce inbounds flag for pointer arithmetic (PR #131862)
efriedma-quic wrote: If an pointer is constructed using inttoptr, it can be based on multiple objects. (In IR, we can see the inttoptr, but in SelectionDAG, it's treated as a noop and eliminated.) The "inbounds" rule should probably say something like this: "The base pointer must be based on one or more allocated objects for which the following applies: the base pointer has an *in bounds* address of the allocated object, and the resulting pointer is *in bounds* of the allocated object." - Actually, thinking about it a bit more, the "allocated object" referenced in the LangRef spec doesn't actually have to be live. So you also have to worry about objects which were previously allocated at the same address... which means inbounds is basically meaningless for a pointer created using inttoptr. https://github.com/llvm/llvm-project/pull/131862 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [Clang][CodeGen] Do not promote if complex divisor is real (PR #131451)
https://github.com/efriedma-quic edited https://github.com/llvm/llvm-project/pull/131451 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [Clang][CodeGen] Do not promote if complex divisor is real (PR #131451)
@@ -314,7 +313,7 @@ class ComplexExprEmitter } QualType getPromotionType(FPOptionsOverride Features, QualType Ty, -bool IsDivOpCode = false) { +bool IsComplexDivisor = false) { efriedma-quic wrote: Maybe delete the `= false`? Default arguments tends to be confusing in cases like this. https://github.com/llvm/llvm-project/pull/131451 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [Clang][CodeGen] Do not promote if complex divisor is real (PR #131451)
https://github.com/efriedma-quic approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/131451 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [SDAG] Introduce inbounds flag for pointer arithmetic (PR #131862)
efriedma-quic wrote: This seems semantically ambiguous. In GlobalISel, you have G_PTR_ADD, and inbounds on that has an obvious meaning; G_PTR_ADD has basically the same semantics as getelementptr. But in SelectionDAG, we don't have that; we just have a plain ISD::ADD. How do you tell which operand is the pointer? The obvious heuristic is outright wrong: you can't assume a value is being used as a pointer just because its value is equal to the address of some variable. https://github.com/llvm/llvm-project/pull/131862 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [Clang][CodeGen] Promote in complex compound divassign (PR #131453)
https://github.com/efriedma-quic approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/131453 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [SDAG] Introduce inbounds flag for pointer arithmetic (PR #131862)
efriedma-quic wrote: Saying "one side is inbounds of the other side" is basically useless, as far as I can tell, for almost any transform. The other possibility you mentioned is that we say one side is a constant, the other is not, and the non-constant side must be a pointer? That seems fragile. The semantics of the add depend on the "syntactic" form of the operands, which could be rewritten. Not really great. Maybe we could consider adding "ISD::PTRADD"? Lowers to ISD::ADD by default, but targets that want to do weird things with pointer arithmetic could do them. One other concern, which applies to basically any formulation of this: Since SelectionDAG doesn't have a distinct pointer type, you can't tell whether the pointer operand was produced by an inttoptr. So in some cases, you have an operation marked "inbounds", but it's ambiguous which object it's actually inbounds to. This isn't really a problem at the moment because we do IR-level transforms that remove inttoptr anyway, but if we ever do resolve the IR-level issues, we should have some idea for how we propagate the fix to SelectionDAG. https://github.com/llvm/llvm-project/pull/131862 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [SDAG] Introduce inbounds flag for pointer arithmetic (PR #131862)
efriedma-quic wrote: > That's for instance useful if your architecture has memory segments whose > borders allocated objects cannot cross and where you can only fold offsets > into memory access instructions if they don't leave the memory segment of the > base address. Hmm, I hadn't really thought about that. I guess that's true. That isn't really usable information on a conventional CPU target, but I guess on a target where the address-space has unusual properties, it could make sense. I think if we add something called "inbounds" to SelectionDAG, it will inevitably get misused for other purposes, though. https://github.com/llvm/llvm-project/pull/131862 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [SDAG] Introduce inbounds flag for pointer arithmetic (PR #131862)
efriedma-quic wrote: If we have PTRADD without a corresponding pointer type, the operand of the PTRADD is implicitly an inttoptr, which causes the problems we're discussing. Which... the IR layer doesn't really properly preserve inttoptr operations in all circumstances, but we're trying to fix that, so I don't want to make the problem worse. https://github.com/llvm/llvm-project/pull/131862 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [llvm][IR] Treat memcmp and bcmp as libcalls (PR #135706)
efriedma-quic wrote: We need to enter the "-fno-builtins" world to make interprocedural optimizations with libc safe. Most optimizations you care about can be preserved in other ways. For example, if malloc called some intrinsic "llvm.allocate_memory"/"llvm.free_memory" to create/destroy provenance, we can preserve most aliasing-related optimizations. If your libc does runtime CPU detection, we can come up with some way to accurately model aliasing on those globals. But we need a different IR representation to make this work; we can't just treat the implementations as opaque. If you want to run certain optimizations before we enter the "-fno-builtins" world, you need some pass that transitions IR from the "builtins" world to the "nobuiltins" world. It might be possible for us to invent a "partial-builtin" mode which treats functions which are called as builtins, but doesn't allow generating calls to functions which aren't already used. Which would allow LTO to accurately to more accurately compute which libc functions are used. But I'm not sure how useful this would actually be in practice; if you're not LTO'ing libc, the dependencies don't really need to be accurate. - There's a smaller set of functions which have more subtle ABI rules: those we call even with -fno-builtins. These are mostly listed in RuntimeLibcalls.def. But memcmp is not one of those functions. https://github.com/llvm/llvm-project/pull/135706 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [llvm][IR] Treat memcmp and bcmp as libcalls (PR #135706)
efriedma-quic wrote: I can join libc monthly meeting, sure. https://github.com/llvm/llvm-project/pull/135706 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [llvm][IR] Treat memcmp and bcmp as libcalls (PR #135706)
efriedma-quic wrote: There are, currently, basically three different ways to supply libc which we support: - Dynamic linking: the libc isn't part of your program at all, it's part of the environment. You only have the abstract interface. - Static linking, no LTO of libc: the libc becomes part of your program at link-time: the linker lowers the libc abstraction into concrete calls. - -fno-builtins: The libc, excluding a small set of functions which are necessary for codegen, becomes part of your program at compile-time; once you're in LLVM IR, the abstraction is gone. To do "LTO of libc", you want something else... but you need to define what "something else" is. There needs to be a clearly delineated point at which libc implementation bits transition from an abstraction to concrete code. If your libc cooperates, that point can be different for different interfaces, but there still needs to be a specific point. You can't just blindly throw everything at the existing optimizer and hope for the best. If you say memcmp stays an abstraction past codegen, you're getting basically zero benefit from LTO'ing it, as far as I can tell: the caller is opaque to the callee, and the callee is opaque to the caller. At best. At worst, your code explodes because your memcmp implementation depends on runtime CPU detection code that runs startup, and LTO can't understand the dependency between the detection code and memcmp. So in essence, I feel like can't review this patch without a proposal that addresses where we're going overall. https://github.com/llvm/llvm-project/pull/135706 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [llvm][IR] Treat memcmp and bcmp as libcalls (PR #135706)
efriedma-quic wrote: I don't really want to start adding functions to RuntimeLibcalls.def piecemeal without documented criteria for what, exactly, should be added. Do we need to add every single function that any transformation can generate under any circumstances? Or is there some criteria we can use to restrict this? I mean, we do memcmp->bcmp, yes, but we also touch a bunch of other math and I/O functions. Do we need to add all the functions from BuildLibCalls.h? Certain libcalls are special because we can generate calls to them even with -fno-builtins: there is no alternative implementation. Like for memcpy, or __stack_chk_fail, or floating-point arithmetic on soft-float targets. memcmp isn't special like this. https://github.com/llvm/llvm-project/pull/135706 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [AMDGPU] Respect MBB alignment in the getFunctionCodeSize() (PR #127142)
@@ -212,6 +212,8 @@ uint64_t SIProgramInfo::getFunctionCodeSize(const MachineFunction &MF) { uint64_t CodeSize = 0; for (const MachineBasicBlock &MBB : MF) { +CodeSize = alignTo(CodeSize, MBB.getAlignment()); efriedma-quic wrote: This is not the right way to do this kind of math; you don't know how much padding the assembler is actually going to insert. Use a conservative estimate like `CodeSize += MBB.getAlignment() - 1`. See https://reviews.llvm.org/D150009 for further discussion. https://github.com/llvm/llvm-project/pull/127142 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [AMDGPU] Respect MBB alignment in the getFunctionCodeSize() (PR #127142)
@@ -212,6 +212,8 @@ uint64_t SIProgramInfo::getFunctionCodeSize(const MachineFunction &MF) { uint64_t CodeSize = 0; for (const MachineBasicBlock &MBB : MF) { +CodeSize = alignTo(CodeSize, MBB.getAlignment()); efriedma-quic wrote: Hmm, okay. Probably worth a comment in the code. https://github.com/llvm/llvm-project/pull/127142 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/20.x: [CUDA] Add support for sm101 and sm120 target architectures (#127187) (PR #127918)
efriedma-quic wrote: The process is that the patch is first reviewed by someone familiar with the code. They approve the patch, and describe how the fix meets the release branch patch requirements (https://llvm.org/docs/HowToReleaseLLVM.html#release-patch-rules). Once it's approved, the release manager will look at the look at the patch, and either merge or request changes. You don't need to specifically ping the release manager; they track all the pending pull requests. https://github.com/llvm/llvm-project/pull/127918 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [SDAG] Introduce inbounds flag for pointer arithmetic (PR #131862)
efriedma-quic wrote: > with respect to whatever the address operand is pointing to Say you have two adjacent objects `a` and `b`. So `&a+1 == &b`. If you have an integer `x` such that `x == &a+1 == &b`, which object is `x` pointing to? In some cases, you might be able to disambiguate based on the RHS of the ptradd; if the offset is positive, it must be inbounds relative to `b`, or else the result would be poison. Or which object it points to might not matter for certain transforms. https://github.com/llvm/llvm-project/pull/131862 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] clang: Fix broken implicit cast to generic address space (PR #138863)
https://github.com/efriedma-quic approved this pull request. https://github.com/llvm/llvm-project/pull/138863 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [KeyInstr][Clang] Add Clang option -g[no-]key-instructions (PR #134627)
@@ -4767,6 +4767,13 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, const llvm::Triple &T, CmdArgs.push_back("-gembed-source"); } + if (Args.hasFlag(options::OPT_gkey_instructions, + options::OPT_gno_key_instructions, false)) { +CmdArgs.push_back("-gkey-instructions"); efriedma-quic wrote: It's not something we encourage. -mllvm flags have significant problems: they're global variables, they go through a separate parser from the regular flag parser, and they usually interact badly with LTO. But in practice, a lot of code does it this way because the "correct" way of adding a flag requires more boilerplate. https://github.com/llvm/llvm-project/pull/134627 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] CodeGen: Fix implementation of __builtin_trivially_relocate. (PR #140312)
@@ -4425,6 +4425,14 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, Address Dest = EmitPointerWithAlignment(E->getArg(0)); Address Src = EmitPointerWithAlignment(E->getArg(1)); Value *SizeVal = EmitScalarExpr(E->getArg(2)); +if (BuiltinIDIfNoAsmLabel == Builtin::BI__builtin_trivially_relocate) + SizeVal = Builder.CreateMul( efriedma-quic wrote: Should this multiply trigger some sort of ubsan check if it overflows? https://github.com/llvm/llvm-project/pull/140312 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/20.x: [Clang][AST] Fix HandleLValueBase to deal with references (#140105) (PR #140246)
https://github.com/efriedma-quic approved this pull request. LGTM; seems like a conservative fix for the crash, should be safe for the branch. https://github.com/llvm/llvm-project/pull/140246 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/20.x: [Clang][AST] Fix HandleLValueBase to deal with references (#140105) (PR #140246)
efriedma-quic wrote: How do we do release notes for things that are fixed on a release branch after the release? Do we put a release note for both 20 and 21? https://github.com/llvm/llvm-project/pull/140246 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] Enable fexec-charset option (PR #138895)
@@ -2191,6 +2243,16 @@ void StringLiteralParser::init(ArrayRef StringToks){ if (CopyStringFragment(StringToks[i], ThisTokBegin, BeforeCRLF)) hadError = true; +if (!hadError && Converter) { + assert(Kind != tok::wide_string_literal && + "Wide character translation not supported"); + SmallString<256> CpConv; + int ResultLength = BeforeCRLF.size() * CharByteWidth; + char *Cp = ResultPtr - ResultLength; + Converter->convert(StringRef(Cp, ResultLength), CpConv); + memcpy(Cp, CpConv.data(), ResultLength); efriedma-quic wrote: Can this overflow the buffer? https://github.com/llvm/llvm-project/pull/138895 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] Enable fexec-charset option (PR #138895)
@@ -249,6 +250,7 @@ static unsigned ProcessCharEscape(const char *ThisTokBegin, case '4': case '5': case '6': case '7': { // Octal escapes. --ThisTokBuf; +Translate = false; efriedma-quic wrote: Also handle `\o` escapes? https://github.com/llvm/llvm-project/pull/138895 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] Add deactivation symbol operand to ConstantPtrAuth. (PR #133537)
https://github.com/efriedma-quic edited https://github.com/llvm/llvm-project/pull/133537 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] Add deactivation symbol operand to ConstantPtrAuth. (PR #133537)
@@ -1699,7 +1699,9 @@ LLVMValueRef LLVMConstantPtrAuth(LLVMValueRef Ptr, LLVMValueRef Key, LLVMValueRef Disc, LLVMValueRef AddrDisc) { return wrap(ConstantPtrAuth::get( unwrap(Ptr), unwrap(Key), - unwrap(Disc), unwrap(AddrDisc))); + unwrap(Disc), unwrap(AddrDisc), + ConstantPointerNull::get( + cast(unwrap(AddrDisc)->getType(); efriedma-quic wrote: Do we want to extend the C API to give access to this? https://github.com/llvm/llvm-project/pull/133537 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] Add deactivation symbol operand to ConstantPtrAuth. (PR #133537)
https://github.com/efriedma-quic commented: Missing verifier checks? https://github.com/llvm/llvm-project/pull/133537 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [AArch64][SME] Support split ZPR and PPR area allocation (PR #142392)
efriedma-quic wrote: In the implementation you're interested in, is there not a hazard between PPRs and GPRs? What's the interaction between this and aarch64-enable-zpr-predicate-spills? https://github.com/llvm/llvm-project/pull/142392 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] ARM: Move ABI helpers from Subtarget to TargetMachine (PR #144680)
https://github.com/efriedma-quic approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/144680 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] release/20.x: [AArch64][SME] Fix accessing the emergency spill slot with hazard padding (#142190) (PR #142741)
https://github.com/efriedma-quic approved this pull request. LGTM I think this makes sense for the branch: it fixes a miscompile with hazard padding, and it's clearly a no-op if hazard padding is disabled. https://github.com/llvm/llvm-project/pull/142741 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] RuntimeLibcalls: Cleanup sincos predicate functions (PR #143081)
https://github.com/efriedma-quic approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/143081 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/20.x: [clang] Don't evaluate the initializer of constexpr-unknown parameters. (#142498) (PR #142648)
https://github.com/efriedma-quic created https://github.com/llvm/llvm-project/pull/142648 Backport 97885213bd4507b204b050c3cd570e365d21cc7d >From c40a997d92a6939e3fb75c6a8766c851c57f3c58 Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Tue, 3 Jun 2025 09:51:37 -0700 Subject: [PATCH] release/20.x: [clang] Don't evaluate the initializer of constexpr-unknown parameters. (#142498) Backport 97885213bd4507b204b050c3cd570e365d21cc7d --- clang/lib/AST/ExprConstant.cpp | 7 ++- clang/test/SemaCXX/constant-expression-p2280r4.cpp | 12 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index e0746f4532245..209b269122a8e 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -3525,7 +3525,12 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E, // should begin within the evaluation of E // Used to be C++20 [expr.const]p5.12.2: // ... its lifetime began within the evaluation of E; - if (isa(VD) && !AllowConstexprUnknown) { + if (isa(VD)) { +if (AllowConstexprUnknown) { + Result = &Info.CurrentCall->createConstexprUnknownAPValues(VD, Base); + return true; +} + // Assume parameters of a potential constant expression are usable in // constant expressions. if (!Info.checkingPotentialConstantExpression() || diff --git a/clang/test/SemaCXX/constant-expression-p2280r4.cpp b/clang/test/SemaCXX/constant-expression-p2280r4.cpp index 87beeb4d3dc84..dbaebb81b93e8 100644 --- a/clang/test/SemaCXX/constant-expression-p2280r4.cpp +++ b/clang/test/SemaCXX/constant-expression-p2280r4.cpp @@ -200,3 +200,15 @@ int f() { return !get_value(); // contextually convert the function call result to bool } } + +namespace param_reference { + constexpr int arbitrary = -12345; + constexpr void f(const int &x = arbitrary) { // expected-note {{declared here}} +constexpr const int &v1 = x; // expected-error {{must be initialized by a constant expression}} \ +// expected-note {{reference to 'x' is not a constant expression}} +constexpr const int &v2 = (x, arbitrary); // expected-warning {{left operand of comma operator has no effect}} +constexpr int v3 = x; // expected-error {{must be initialized by a constant expression}} +static_assert(x==arbitrary); // expected-error {{static assertion expression is not an integral constant expression}} +static_assert(&x - &x == 0); + } +} ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [AArch64][SME] Support split ZPR and PPR area allocation (PR #142392)
https://github.com/efriedma-quic edited https://github.com/llvm/llvm-project/pull/142392 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [AArch64][SME] Support split ZPR and PPR area allocation (PR #142392)
https://github.com/efriedma-quic commented: If you express the size of the hazard padding between the PPRs and ZPRs as a scalable size, that might simplify some of the logic? You wouldn't need to represent the two areas as separate stacks, at least. Maybe it's also worth considering "preallocating" some stack slots, along the lines of LocalStackSlotAllocation; anything we can allocate in an earlier pass doesn't have to be part of the core stack layout, and we probably get better code if we have a "base" for ZPR variables. Do we care at all how stack protectors fit into all of this? I suspect the stack protector ends up in an unfortunate position by default. https://github.com/llvm/llvm-project/pull/142392 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [AArch64][SME] Support split ZPR and PPR area allocation (PR #142392)
@@ -3780,25 +3938,49 @@ void AArch64FrameLowering::determineStackHazardSlot( bool HasFPRCSRs = any_of(SavedRegs.set_bits(), [](unsigned Reg) { return AArch64::FPR64RegClass.contains(Reg) || AArch64::FPR128RegClass.contains(Reg) || - AArch64::ZPRRegClass.contains(Reg) || - AArch64::PPRRegClass.contains(Reg); + AArch64::ZPRRegClass.contains(Reg); + }); + bool HasPPRCSRs = any_of(SavedRegs.set_bits(), [](unsigned Reg) { +return AArch64::PPRRegClass.contains(Reg); }); bool HasFPRStackObjects = false; - if (!HasFPRCSRs) { -std::vector FrameObjects(MFI.getObjectIndexEnd()); + bool HasPPRStackObjects = false; + if (!HasFPRCSRs || SplitSVEObjects) { +enum SlotType : uint8_t { + Unknown = 0, + ZPRorFPR = 1 << 0, + PPR = 1 << 1, + GPR = 1 << 2, + LLVM_MARK_AS_BITMASK_ENUM(GPR) +}; + +// Find stack slots solely used for one kind of register (ZPR, PPR, etc.), +// based on the kinds of accesses used in the function. +SmallVector SlotTypes(MFI.getObjectIndexEnd(), SlotType::Unknown); for (auto &MBB : MF) { for (auto &MI : MBB) { std::optional FI = getLdStFrameID(MI, MFI); -if (FI && *FI >= 0 && *FI < (int)FrameObjects.size()) { - if (MFI.isScalableStackID(*FI) || AArch64InstrInfo::isFpOrNEON(MI)) -FrameObjects[*FI] |= 2; - else -FrameObjects[*FI] |= 1; +if (!FI || FI < 0 || FI > int(SlotTypes.size())) + continue; +bool IsScalable = MFI.isScalableStackID(*FI); +bool IsPPR = IsScalable && isPPRAccess(MI); +if (IsScalable || AArch64InstrInfo::isFpOrNEON(MI)) { + SlotTypes[*FI] |= IsPPR ? SlotType::PPR : SlotType::ZPRorFPR; +} else { + SlotTypes[*FI] |= SlotType::GPR; } } } -HasFPRStackObjects = -any_of(FrameObjects, [](unsigned B) { return (B & 3) == 2; }); + +for (int FI = 0; FI < int(SlotTypes.size()); ++FI) { + HasFPRStackObjects |= SlotTypes[FI] == SlotType::ZPRorFPR; + // For SplitSVEObjects remember that this stack slot is a predicate, this + // will be needed later when determining the frame layout. + if (SlotTypes[FI] == SlotType::PPR) { efriedma-quic wrote: This doesn't look quite right. Anything with scalable size needs to be either ScalablePredVector or ScalableVector. Anything with non-scalable size needs to be on the normal stack, I think? https://github.com/llvm/llvm-project/pull/142392 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [AArch64][SME] Support split ZPR and PPR area allocation (PR #142392)
@@ -57,7 +57,8 @@ class AArch64FrameLowering : public TargetFrameLowering { StackOffset resolveFrameOffsetReference(const MachineFunction &MF, int64_t ObjectOffset, bool isFixed, bool isSVE, Register &FrameReg, - bool PreferFP, bool ForSimm) const; + bool PreferFP, bool ForSimm, + int64_t FI = -1) const; efriedma-quic wrote: resolveFrameOffsetReference only has three callers; I'm not sure you're getting much by using a default argument here. https://github.com/llvm/llvm-project/pull/142392 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [AArch64][SME] Support split ZPR and PPR area allocation (PR #142392)
@@ -3780,25 +3938,49 @@ void AArch64FrameLowering::determineStackHazardSlot( bool HasFPRCSRs = any_of(SavedRegs.set_bits(), [](unsigned Reg) { return AArch64::FPR64RegClass.contains(Reg) || AArch64::FPR128RegClass.contains(Reg) || - AArch64::ZPRRegClass.contains(Reg) || - AArch64::PPRRegClass.contains(Reg); + AArch64::ZPRRegClass.contains(Reg); + }); + bool HasPPRCSRs = any_of(SavedRegs.set_bits(), [](unsigned Reg) { +return AArch64::PPRRegClass.contains(Reg); }); bool HasFPRStackObjects = false; - if (!HasFPRCSRs) { -std::vector FrameObjects(MFI.getObjectIndexEnd()); + bool HasPPRStackObjects = false; + if (!HasFPRCSRs || SplitSVEObjects) { +enum SlotType : uint8_t { + Unknown = 0, + ZPRorFPR = 1 << 0, + PPR = 1 << 1, + GPR = 1 << 2, + LLVM_MARK_AS_BITMASK_ENUM(GPR) +}; + +// Find stack slots solely used for one kind of register (ZPR, PPR, etc.), +// based on the kinds of accesses used in the function. +SmallVector SlotTypes(MFI.getObjectIndexEnd(), SlotType::Unknown); for (auto &MBB : MF) { for (auto &MI : MBB) { std::optional FI = getLdStFrameID(MI, MFI); -if (FI && *FI >= 0 && *FI < (int)FrameObjects.size()) { - if (MFI.isScalableStackID(*FI) || AArch64InstrInfo::isFpOrNEON(MI)) -FrameObjects[*FI] |= 2; - else -FrameObjects[*FI] |= 1; +if (!FI || FI < 0 || FI > int(SlotTypes.size())) + continue; +bool IsScalable = MFI.isScalableStackID(*FI); +bool IsPPR = IsScalable && isPPRAccess(MI); +if (IsScalable || AArch64InstrInfo::isFpOrNEON(MI)) { + SlotTypes[*FI] |= IsPPR ? SlotType::PPR : SlotType::ZPRorFPR; +} else { + SlotTypes[*FI] |= SlotType::GPR; } } } -HasFPRStackObjects = -any_of(FrameObjects, [](unsigned B) { return (B & 3) == 2; }); + +for (int FI = 0; FI < int(SlotTypes.size()); ++FI) { + HasFPRStackObjects |= SlotTypes[FI] == SlotType::ZPRorFPR; + // For SplitSVEObjects remember that this stack slot is a predicate, this + // will be needed later when determining the frame layout. + if (SlotTypes[FI] == SlotType::PPR) { efriedma-quic wrote: Oh, I got confused by this bit: ``` if (IsScalable || AArch64InstrInfo::isFpOrNEON(MI)) { SlotTypes[*FI] |= IsPPR ? SlotType::PPR : SlotType::ZPRorFPR; } else { SlotTypes[*FI] |= SlotType::GPR; } ``` Maybe rewrite it as the following, to split the cases a bit more cleanly? ``` if (IsScalable) { SlotTypes[*FI] |= isPPRAccess(MI) ? SlotType::PPR : SlotType::ZPRorFPR; } else { SlotTypes[*FI] |= AArch64InstrInfo::isFpOrNEON(MI) ? SlotType::ZPRorFPR : SlotType::GPR; } ``` Also, I'm not sure isFpOrNEON is quite sufficient here; we probably also want to handle SVE accesses to fixed-width slots. https://github.com/llvm/llvm-project/pull/142392 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] RuntimeLibcalls: Use array initializers for default values (PR #143082)
@@ -103,10 +103,14 @@ struct RuntimeLibcallsInfo { private: /// Stores the name each libcall. - const char *LibcallRoutineNames[RTLIB::UNKNOWN_LIBCALL + 1]; + const char *LibcallRoutineNames[RTLIB::UNKNOWN_LIBCALL + 1] = {nullptr}; + + static_assert(static_cast(CallingConv::C) == 0, +"default calling conv should be encoded as 0"); /// Stores the CallingConv that should be used for each libcall. - CallingConv::ID LibcallCallingConvs[RTLIB::UNKNOWN_LIBCALL]; + CallingConv::ID LibcallCallingConvs[RTLIB::UNKNOWN_LIBCALL] = { + CallingConv::C}; efriedma-quic wrote: Explicitly writing "CallingConv::C" here is confusing; either just use `{}`, or use something like SmallVector that actually has a constructor which fills an arbitrary value. https://github.com/llvm/llvm-project/pull/143082 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/20.x: [clang] Don't evaluate the initializer of constexpr-unknown parameters. (#142498) (PR #142648)
efriedma-quic wrote: Didn't realize this was still waiting for review... added a couple more reviewers. https://github.com/llvm/llvm-project/pull/142648 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] RuntimeLibcalls: Use array initializers for default values (PR #143082)
https://github.com/efriedma-quic approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/143082 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [AArch64][SME] Support Windows/stack probes in MachineSMEABIPass (PR #149063)
efriedma-quic wrote: I'll restate: On Windows, there are guard pages, so you have to grow the stack one page at a time. There are two ways to do this: __chkstk, or explicit memory accesses. __chkstk is faster for large allocations because it caches the size of the stack. LLVM currently relies explicit accesses for prologues that allocate less than one page, and __chkstk for all other allocations. But the ABI doesn't require that; we can use whatever mix we want. https://github.com/llvm/llvm-project/pull/149063 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits