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-co
@@ -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)));
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
@@ -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/
@@ -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 &&
+
@@ -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));
+
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-b
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@li
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
@@ -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)) {
+CmdA
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
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 pro
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
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
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 fi
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 th
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
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 o
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 fragi
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-commi
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
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-commi
@@ -314,7 +313,7 @@ class ComplexExprEmitter
}
QualType getPromotionType(FPOptionsOverride Features, QualType Ty,
-bool IsDivOpCode = false) {
+bool IsComplexDivisor = false) {
efriedma-quic wrote:
M
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
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 rele
@@ -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
@@ -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 righ
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-
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-commit
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/ll
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 conserv
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
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-commi
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@
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 refactor
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
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
_
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
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
@@ -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-branc
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-bra
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
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/pu
@@ -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 = Builde
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-comm
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-commi
@@ -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
@@ -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
@@ -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
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.) S
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
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-commi
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-commi
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-commi
@@ -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 MergePrivateGloba
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
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
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 specific
@@ -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
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
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
@@ -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``
+^
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:
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-com
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 th
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.or
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.co
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.or
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
___
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
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
_
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 in
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
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/listi
@@ -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(F
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 m
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-A
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
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-commit
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-comm
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/l
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
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-comm
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/t2L
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 w
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
86 matches
Mail list logo
