[llvm-branch-commits] [clang] [clang] Define ptrauth_sign_constant builtin. (PR #93904)

2024-05-30 Thread Eli Friedman via llvm-branch-commits

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)

2024-06-03 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2024-06-21 Thread Eli Friedman via llvm-branch-commits

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)

2024-06-27 Thread Eli Friedman via llvm-branch-commits

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)

2024-06-27 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2024-06-27 Thread Eli Friedman via llvm-branch-commits

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)

2024-07-29 Thread Eli Friedman via llvm-branch-commits

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)

2024-07-30 Thread Eli Friedman via llvm-branch-commits

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)

2024-07-30 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2024-08-08 Thread Eli Friedman via llvm-branch-commits

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)

2024-08-08 Thread Eli Friedman via llvm-branch-commits

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)

2024-08-13 Thread Eli Friedman via llvm-branch-commits

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)

2024-08-14 Thread Eli Friedman via llvm-branch-commits

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)

2024-08-15 Thread Eli Friedman via llvm-branch-commits

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)

2024-02-14 Thread Eli Friedman via llvm-branch-commits

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)

2024-02-28 Thread Eli Friedman via llvm-branch-commits

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)

2024-03-11 Thread Eli Friedman via llvm-branch-commits

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)

2024-03-11 Thread Eli Friedman via llvm-branch-commits

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)

2024-03-11 Thread Eli Friedman via llvm-branch-commits

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)

2024-03-11 Thread Eli Friedman via llvm-branch-commits

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)

2024-03-11 Thread Eli Friedman via llvm-branch-commits

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)

2024-03-26 Thread Eli Friedman via llvm-branch-commits

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)

2024-04-05 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2024-04-08 Thread Eli Friedman via llvm-branch-commits

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)

2024-04-16 Thread Eli Friedman via llvm-branch-commits

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)

2024-04-19 Thread Eli Friedman via llvm-branch-commits

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)

2024-04-24 Thread Eli Friedman via llvm-branch-commits

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)

2024-04-25 Thread Eli Friedman via llvm-branch-commits

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)

2024-04-29 Thread Eli Friedman via llvm-branch-commits

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)

2024-04-30 Thread Eli Friedman via llvm-branch-commits

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)

2024-05-08 Thread Eli Friedman via llvm-branch-commits

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)

2024-05-11 Thread Eli Friedman via llvm-branch-commits

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)

2024-05-16 Thread Eli Friedman via llvm-branch-commits

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)

2024-05-17 Thread Eli Friedman via llvm-branch-commits

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)

2023-12-12 Thread Eli Friedman via llvm-branch-commits

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)

2023-12-12 Thread Eli Friedman via llvm-branch-commits

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)

2024-08-22 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2024-08-22 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2024-08-22 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2024-08-28 Thread Eli Friedman via llvm-branch-commits

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)

2024-09-04 Thread Eli Friedman via llvm-branch-commits

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.

2019-12-09 Thread Eli Friedman via llvm-branch-commits

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)

2024-10-14 Thread Eli Friedman via llvm-branch-commits

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)

2024-10-15 Thread Eli Friedman via llvm-branch-commits

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)

2024-10-09 Thread Eli Friedman via llvm-branch-commits

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)

2024-10-09 Thread Eli Friedman via llvm-branch-commits

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)

2024-10-11 Thread Eli Friedman via llvm-branch-commits

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)

2024-10-04 Thread Eli Friedman via llvm-branch-commits

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)

2024-10-04 Thread Eli Friedman via llvm-branch-commits

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)

2024-10-04 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2024-10-04 Thread Eli Friedman via llvm-branch-commits

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)

2024-09-26 Thread Eli Friedman via llvm-branch-commits

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)

2024-09-26 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2024-09-26 Thread Eli Friedman via llvm-branch-commits

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)

2024-11-14 Thread Eli Friedman via llvm-branch-commits

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)

2025-02-05 Thread Eli Friedman via llvm-branch-commits

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)

2025-02-11 Thread Eli Friedman via llvm-branch-commits

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)

2025-01-30 Thread Eli Friedman via llvm-branch-commits

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)

2025-01-10 Thread Eli Friedman via llvm-branch-commits

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)

2025-03-21 Thread Eli Friedman via llvm-branch-commits

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)

2025-03-17 Thread Eli Friedman via llvm-branch-commits

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)

2025-03-17 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2025-03-17 Thread Eli Friedman via llvm-branch-commits

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)

2025-03-18 Thread Eli Friedman via llvm-branch-commits

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)

2025-03-18 Thread Eli Friedman via llvm-branch-commits

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)

2025-03-19 Thread Eli Friedman via llvm-branch-commits

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)

2025-03-28 Thread Eli Friedman via llvm-branch-commits

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)

2025-03-31 Thread Eli Friedman via llvm-branch-commits

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)

2025-04-16 Thread Eli Friedman via llvm-branch-commits

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)

2025-05-05 Thread Eli Friedman via llvm-branch-commits

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)

2025-04-15 Thread Eli Friedman via llvm-branch-commits

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)

2025-04-15 Thread Eli Friedman via llvm-branch-commits

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)

2025-02-13 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2025-02-13 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2025-02-21 Thread Eli Friedman via llvm-branch-commits

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)

2025-03-20 Thread Eli Friedman via llvm-branch-commits

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)

2025-05-07 Thread Eli Friedman via llvm-branch-commits

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)

2025-05-06 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2025-05-16 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2025-05-16 Thread Eli Friedman via llvm-branch-commits

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)

2025-05-16 Thread Eli Friedman via llvm-branch-commits

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)

2025-05-19 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2025-05-19 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2025-05-27 Thread Eli Friedman via llvm-branch-commits

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)

2025-05-27 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2025-05-27 Thread Eli Friedman via llvm-branch-commits

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)

2025-06-02 Thread Eli Friedman via llvm-branch-commits

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)

2025-06-18 Thread Eli Friedman via llvm-branch-commits

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)

2025-06-09 Thread Eli Friedman via llvm-branch-commits

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)

2025-06-09 Thread Eli Friedman via llvm-branch-commits

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)

2025-06-03 Thread Eli Friedman via llvm-branch-commits

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)

2025-06-03 Thread Eli Friedman via llvm-branch-commits

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)

2025-06-03 Thread Eli Friedman via llvm-branch-commits

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)

2025-06-03 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2025-06-03 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2025-06-04 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2025-06-06 Thread Eli Friedman via llvm-branch-commits


@@ -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)

2025-06-11 Thread Eli Friedman via llvm-branch-commits

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)

2025-06-09 Thread Eli Friedman via llvm-branch-commits

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)

2025-07-16 Thread Eli Friedman via llvm-branch-commits

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


  1   2   >