[PATCH] D78747: [SveEmitter] Add builtins for compares and ReverseCompare flag.

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM with one minor comment Comment at: clang/include/clang/Basic/arm_sve.td:74 // +// w: vector of element type promoted to 64bits +// j: element type promoted to 64bit

[PATCH] D78747: [SveEmitter] Add builtins for compares and ReverseCompare flag.

efriedma added inline comments. Comment at: clang/include/clang/Basic/arm_sve.td:74 // +// w: vector of element type promoted to 64bits +// j: element type promoted to 64bits (splat to vector type) Isn't "w" the same as the existing "g"? Or is there some differ

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

efriedma added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:2928 + // FIXME: This does not find the VLA if it is embedded in other types, + // like here: `void (*vla)(int[x]);` for (const VariableArrayType* VA = FindVA(VD->getType().getTypePtr());

[PATCH] D78569: [SVE][CodeGen] Lower SDIV & UDIV to SVE intrinsics

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:11388 return LowerSVEIntrinsicEXT(N, DAG); +case Intrinsic::aarch64_sve_sdiv: +return DAG.ge

[PATCH] D78674: [SveEmitter] Add builtins for contiguous prefetches

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM with a couple minor comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7727 + Value *Offset = Ops.size() > 3 ? Ops[2] : Builder.getInt32(0); + BasePtr = Builder

[PATCH] D78673: [SveEmitter] Use llvm.aarch64.sve.ld1/st1 for contiguous load/store builtins

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78673/new/ https://reviews.llvm.org/D78673 ___

[PATCH] D78569: [SVE][CodeGen] Lower SDIV & UDIV to SVE intrinsics

efriedma added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:7670 + Mask, Op.getOperand(0), Op.getOperand(1)); +} + sdesmalen wrote: > efriedma wrote: > > If we're going to support these operations, we might as w

[PATCH] D78569: [SVE][CodeGen] Lower SDIV & UDIV to SVE intrinsics

efriedma added a comment. I'd prefer to handle legalization in a separate patch from handling legal sdiv/udiv operations, so we actually have some context to discuss the legalization strategy. Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:7670 +

[PATCH] D78443: [Local] Update getOrEnforceKnownAlignment/getKnownAlignment to use Align/MaybeAlign.

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78443/new/ https://reviews.llvm.org/D78443 ___

[PATCH] D78443: [Local] Update getOrEnforceKnownAlignment/getKnownAlignment to use Align/MaybeAlign.

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:4181 +llvm::getOrEnforceKnownAlignment(V, + llvm::Align(Align.getQuantity()), + *TD) < Align.getQuantity())

[PATCH] D78239: [SveEmitter] Add builtins for FP conversions

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/utils/TableGen/SveEmitter.cpp:1035 + OS << "#define svcvtxnt_f32_x svcvtxnt_f32_m\n"; + OS << "#define svcvtxnt_f32_f64_x svcvtxnt_f32_f64_m\

[PATCH] D78402: [AArch64] Define ACLE FP conversion intrinsics with more specific predicate.

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78402/new/ https://reviews.llvm.org/D78402 ___

[PATCH] D77278: Remove asserting getters from base Type

efriedma accepted this revision. efriedma added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77278/new/ https://reviews.llvm.org/D77278 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D78239: [SveEmitter] Add builtins for FP conversions

efriedma added a comment. I think the predicate type is something we should fix. Even if it's not causing problems now, it seems like the sort of thing that will bite us later. It doesn't necessarily need to block this patch, I guess, but it at least needs a good comment in EmitAArch64SVEBuil

[PATCH] D77597: [SveEmitter] Add ExpandOp1SVALL and builtin for svptrue

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7683 +if (TypeFlags.isExpandOp1SVALL()) { + if (Ops.size() <= 1) +Ops.push_back(Builder.getInt32(31)); sdesmalen wrote: > efriedma wrote: > > The `Ops.size() <= 1` seems

[PATCH] D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally

efriedma added a comment. If you can `assert(ParentCGF.LocalDeclMap.size() == 2);`, I guess the current code is good enough. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77982/new/ https://reviews.llvm.org/D77982 __

[PATCH] D77597: [SveEmitter] Add ExpandOp1SVALL and builtin for svptrue

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7683 +if (TypeFlags.isExpandOp1SVALL()) { + if (Ops.size() <= 1) +Ops.push_back(Builder.getInt32(31)); The `Ops.size() <= 1` seems to return the same result for all the i

[PATCH] D78239: [SveEmitter] Add builtins for FP conversions

efriedma added a comment. > as the LLVM IR intrinsics use the as the predicate. Why are the fp conversion intrinsics special here? Should we fix the LLVM intrinsic definitions? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78239/new/ https://re

[PATCH] D77593: [SveEmitter] Implement zeroing of false lanes

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

[PATCH] D78238: [SveEmitter] Add builtins for svwhilerw/svwhilewr

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78238/new/ https://reviews.llvm.org/D78238 ___

[PATCH] D77596: [SveEmitter] Add IsOverloadNone flag and builtins for svpfalse and svcnt[bhwd]_pat

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

[PATCH] D77595: [SveEmitter] Add builtins for svwhile

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

[PATCH] D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally

efriedma added a comment. For (1), I can see your point that it's sort of a balancing act. But generally, I'm concerned about making fragile assumptions: here, that LocalDeclMap contains precisely the two ImplicitParmDecls for the arguments, and nothing else. If we are going to assume that, I

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

efriedma added a comment. > Where is the size expression actually evaluated? Is it evaluated at the point > of the typedef or at the point of the variable definition? At the point of the typedef. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77809

[PATCH] D78204: [AArch64][SVE] Remove LD1/ST1 dependency on llvm.masked.load/store

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. Practically, I'm not sure you're really getting much benefit out of this; there's very little common code that touches MLOAD/MSTORE nodes anyway. But, sure, LGTM. Repository: rG LLVM

[PATCH] D78162: [CodeGen] Mark inline definitions of builtins as nobuiltin only if we plan to emit them.

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78162/new/ https://reviews.llvm.org/D78162 ___

[PATCH] D78148: [CodeGen] make isTriviallyRecursive handle more trivial recursion

efriedma added a comment. Could you go into a little more detail what problem you're trying to resolve? isTriviallyRecursive is specifically a narrow hack to handle weird cases where a function is trying to hide the fact that it's calling itself, in ways that would convince gcc that the called

[PATCH] D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally

efriedma added a comment. Searching LocalDeclMap is less problematic, I guess... but still, it should be possible to something more straightforward. Maybe make startOutlinedSEHHelper store the actual ImplicitParamDecl, or something like that. Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

efriedma added a comment. That fix looks right; thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77984/new/ https://reviews.llvm.org/D77984 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

This revision was automatically updated to reflect the committed changes. Closed by commit rG89e0662dee5f: Make IRBuilder automatically set alignment on load/store/alloca. (authored by efriedma). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77984/ne

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

efriedma updated this revision to Diff 257063. efriedma added a comment. Actually address all the review comments. Fix CreateAlloca to use the pref alignment instead of the ABI alignment, like instcombine and selectiondag. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

efriedma planned changes to this revision. efriedma added a comment. Err, didn't quite address all the review comments. New version soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77984/new/ https://reviews.llvm.org/D77984 __

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

efriedma updated this revision to Diff 257059. efriedma added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77984/new/ https://reviews.llvm.org/D77984 Files: clang/test/CodeGen/arm_neon_intrinsics.c llvm/incl

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77936/new/ https://reviews.llvm.org/D77936 ___

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

efriedma marked an inline comment as done. efriedma added inline comments. Comment at: llvm/include/llvm/IR/IRBuilder.h:1600 +return CreateAlignedLoad(Ty, Ptr, DL.getABITypeAlign(Ty), isVolatile, Name); } jdoerfert wrote: > Can't we just pawn of the ali

[PATCH] D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally

efriedma added a comment. Again, using the name isn't reliable. Among other things, in release builds, IR values don't have names at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77982/new/ https://reviews.llvm.org/D77982

[PATCH] D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally

efriedma added a comment. Please upload patches with full context (-U100). Comment at: clang/lib/CodeGen/CGException.cpp:1798 + +// if the parent is a _finally, need to retrive Establisher's FP, +// 2nd paramenter, saved & named frame_pointer in parent's frame

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

efriedma created this revision. efriedma added reviewers: jdoerfert, lebedev.ri, spatel. Herald added subscribers: cfe-commits, kerbowa, nhaehnle, jvesely. Herald added a reviewer: bollu. Herald added a project: clang. This is equivalent in terms of LLVM IR semantics, but we want to transition aw

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

efriedma added a comment. I'm not concerned about the performance implications of whatever approach we take here. In the worst case, an "icmp+zext" corresponds to two extra arithmetic instructions; that's not enough to matter. And I expect usually it'll get optimized away. I'd prefer to avoi

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

efriedma added a comment. Instead of asserting there are less than 256 cleanup destinations, can you emit an icmp against zero, or something like that? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77936/new/ https://reviews.llvm.org/D77936 ___

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGException.cpp:1651 + llvm::Value* Load = CGF.Builder.CreateLoad(Addr, "cleanup.dest"); + IsForEH = CGF.Builder.CreateTrunc(Load, CGM.Int8Ty); +} Is just truncating the value really corr

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

efriedma added a comment. Needs a testcase in clang/test/CodeGen to verify the generated IR. I haven't looked at this code in a while, but this looks reasonable. Comment at: clang/lib/CodeGen/EHScopeStack.h:164 +F_IsEHCleanupKind = 0x4, +F_HasSehAbnormalExi

[PATCH] D77903: [SVE] Remove calls to getBitWidth from clang

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77903/new/ https://reviews.llvm.org/D77903 ___

[PATCH] D77257: Clean up usages of asserting vector getters in Type

efriedma accepted this revision. efriedma added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77257/new/ https://reviews.llvm.org/D77257 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D77257: Clean up usages of asserting vector getters in Type

efriedma added inline comments. Comment at: clang/lib/CodeGen/PatternInit.cpp:60 unsigned BitWidth = llvm::APFloat::semanticsSizeInBits( -(Ty->isVectorTy() ? Ty->getVectorElementType() : Ty) +(Ty->isVectorTy() ? cast(Ty)->getElementType() : Ty) -

[PATCH] D76269: [opaque pointer types] Remove deprecated Instruction/IRBuilder APIs.

This revision was automatically updated to reflect the committed changes. Closed by commit rG836ce9db7f13: [opaque pointer types] Remove deprecated Instruction/IRBuilder APIs. (authored by efriedma). Changed prior to commit: https://reviews.llvm.org/D76269?vs=254971&id=256395#toc Repository:

[PATCH] D76078: [AArch64][SVE] Add a pass for SVE intrinsic optimisations

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

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

efriedma removed a reviewer: eli.friedman. efriedma added a comment. Looks roughly fine to me, but I'm not an appropriate reviewer for static analyzer code. Comment at: clang/lib/Analysis/CFG.cpp:2859 // Of everything that can be declared in a DeclStmt, only VarDecls impa

[PATCH] D76269: [opaque pointer types] Remove deprecated Instruction/IRBuilder APIs.

efriedma added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76269/new/ https://reviews.llvm.org/D76269 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D75661: Remove SequentialType from the type heirarchy.

efriedma marked an inline comment as done. efriedma added inline comments. Comment at: mlir/lib/Target/LLVMIR/ModuleTranslation.cpp:60 + } else { emitError(loc) << "expected sequential LLVM types wrapping a scalar"; return nullptr; ftynse wrote: > Nit:

[PATCH] D75661: Remove SequentialType from the type heirarchy.

This revision was automatically updated to reflect the committed changes. Closed by commit rG68b03aee1a15: Remove SequentialType from the type heirarchy. (authored by efriedma). Changed prior to commit: https://reviews.llvm.org/D75661?vs=252659&id=255546#toc Repository: rG LLVM Github Monore

[PATCH] D76680: [SveEmitter] Add immediate checks for lanes and complex imms

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

[PATCH] D77591: [SveEmitter] Explicitly merge with zero/undef

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77591/new/ https://reviews.llvm.org/D77591 ___

[PATCH] D77596: [SveEmitter] Add NoOverload flag and builtin for svpfalse

efriedma added a comment. Would it make sense to generalize getSVEType() to getSVETypeList()? It seems like the current approach won't generalize well once you're dealing with more than one overloaded type (for example, llvm.aarch64.sve.scvtf.nxv8f16.nxv8i16). Comment at: cl

[PATCH] D77593: [SveEmitter] Implement zeroing of false lanes

efriedma added a comment. Maybe better to emit llvm.aarch64.sve.sel for now, if you're trying to avoid IR operations. Otherwise looks fine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77593/new/ https://reviews.llvm.org/D77593 __

[PATCH] D76680: [SveEmitter] Add immediate checks for lanes and complex imms

efriedma added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9208 +def err_rotation_argument_to_cmla +: Error<"argument should be the value 0,90,180 or 270">; def warn_neon_vector_initializer_non_portable : Warning< SjoerdMeij

[PATCH] D76678: [SveEmitter] Add range checks for immediates and predicate patterns.

efriedma accepted this revision. efriedma added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76678/new/ https://reviews.llvm.org/D76678 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/

[PATCH] D76269: [opaque pointer types] Remove deprecated Instruction/IRBuilder APIs.

efriedma updated this revision to Diff 254971. efriedma added a comment. Herald added a project: LLVM. Commited the non-header changes separately Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76269/new/ https://reviews.llvm.org/D76269 Files: llv

[PATCH] D77054: [AArch64][SVE] Add SVE intrinsics for saturating add & subtract

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

[PATCH] D77335: [AST] clang::VectorType supports any size (that fits in unsigned)

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77335/new/ https://reviews.llvm.org/D77335 ___

[PATCH] D77335: [AST] clang::VectorType supports any size (that fits in unsigned)

efriedma added a comment. If you're going to add code to check for it, we might as well add testcases for ridiculous sizes, like `(__int128_t)1 << 100`. I think this makes sense. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77335/new/ https://re

[PATCH] D77054: [AArch64][SVE] Add SVE intrinsics for saturating add & subtract

efriedma added a comment. You should be able to refactor the patterns into the definitions of the multiclasses sve_int_bin_cons_arit_0 and sve_int_arith_imm0, to avoid repeating them four times. (You might want to look at other places using null_frag in SVEInstrFormats.td for inspiration.) C

[PATCH] D77313: [AST] Allow VectorTypes of 1-256 elements, and powers of two up to 2**31.

efriedma added a comment. I think I would rather just pay the extra 8 bytes per VectorType, and expand this to support all vector types supported by LLVM. It's not like we allocate enough VectorTypes for it to matter, anyway. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77236: [SVE] Cleanup prior to introdution of FixedVectorType

efriedma added a comment. My thoughts, in no particular order: 1. This is orthogonal to splitting VectorType, as far as I can tell. `Ty->getVectorNumElements()` works equally well whether the implementation asserts it's a VectorType that isn't scalable, or asserts it's a FixedVectorType. 2. T

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

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

[PATCH] D77048: [Clang][CodeGen] Fixing mismatch between memory layout and const expressions for oversized bitfields

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77048/new/ https://reviews.llvm.org/D77048 ___

[PATCH] D72467: Remove "mask" operand from shufflevector.

efriedma added a comment. (Also merged followup https://github.com/llvm/llvm-project/commit/ba4764c2cc14b0b495af539a913de10cf8268420 to fix a memory leak caught by the bots.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72467/new/ https://review

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

efriedma added a comment. I'd prefer not to revert a change that's three months old without some sort of evidence the issue is actually the compiler's fault. If nobody else has seen an issue, it's probably okay to let it sit for a day or two. Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D77048: [Clang][CodeGen] Fixing mismatch between memory layout and const expressions for oversized bitfields

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGExprConstant.cpp:613 + + // Add padding bits in case of over-sized bit-field. + // "The first sizeof(T)*8 bits are used to hold the value of the bit-field, The existing code in ConstantAggregat

[PATCH] D77056: RFC: [Sema][SVE] Allow non-member operators for SVE types

efriedma added a comment. > One option would to be wait until there's at least one type that > isSizelessBuiltinType but isn't an SVE type. Another would be to introduce it > when a feature seems too SVE-specific Probably not a big deal either way, as long as we document the intent in the code

[PATCH] D77054: [AArch64][SVE] Add SVE intrinsics for saturating add & subtract

efriedma added a comment. I can understand why you might want the new intrinsics as a temporary measure, but I don't see the point of removing the already working support for llvm.sadd. etc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77054/new/

[PATCH] D76943: [clang analysis] Make mutex guard detection more reliable.

This revision was automatically updated to reflect the committed changes. Closed by commit rG24485aec4750: [clang analysis] Make mutex guard detection more reliable. (authored by efriedma). Changed prior to commit: https://reviews.llvm.org/D76943?vs=253253&id=253657#toc Repository: rG LLVM G

[PATCH] D77056: RFC: [Sema][SVE] Allow non-member operators for SVE types

efriedma added a comment. I'm concerned we're going to run into trouble if two people define different SVE "libraries", and you try to link both of them into the same program. If you're not careful, you end up with ODR violations. The scope of this is sort of restricted in normal C++: class a

[PATCH] D76943: [clang analysis] Make mutex guard detection more reliable.

efriedma updated this revision to Diff 253253. efriedma added a comment. Add support for conversion operators. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76943/new/ https://reviews.llvm.org/D76943 Files: clang/lib/Analysis/ThreadSafety.cpp

[PATCH] D76943: [clang analysis] Make mutex guard detection more reliable.

efriedma created this revision. efriedma added a reviewer: rsmith. Herald added a project: clang. -Wthread-safety was failing to detect certain AST patterns it should detect. Make the pattern detection a bit more comprehensive. Due to an unrelated bug involving template instantiation, this showe

[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

efriedma added subscribers: hfinkel, jdoerfert. efriedma added a comment. That makes sense. I would slightly lean towards not generating the assumptions, given the current state of assumptions and the likely benefit in this context. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D75661: Remove SequentialType from the type heirarchy.

efriedma updated this revision to Diff 252659. efriedma added a comment. Herald added subscribers: Joonsoo, liufengdb, lucyrfox, mgester, arpith-jacob, csigg, nicolasvasilache, antiagainst, shauheen, burmako, jpienaar, rriddle, mehdi_amini. Fix bug in GlobalOpt. Add fixes for MLIR. Repository

[PATCH] D75661: Remove SequentialType from the type heirarchy.

efriedma updated this revision to Diff 252645. efriedma added a comment. Address lint comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75661/new/ https://reviews.llvm.org/D75661 Files: clang/lib/CodeGen/CGExprConstant.cpp llvm/include/ll

[PATCH] D76691: [AST][SVE] Treat built-in SVE types as trivially copyable

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76691/new/ https://reviews.llvm.org/D76691 ___

[PATCH] D76690: [AST][SVE] Treat built-in SVE types as POD

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

[PATCH] D76692: [AST][SVE] Treat built-in SVE types as trivial

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

[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

efriedma added a comment. If it's just tramp3d-v4, I'm not that concerned... but that's a weird result. On x86 in particular, alignment markings have almost no effect on optimization, generally. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74183

[PATCH] D76693: [Sema][SVE] Allow ?: to select between SVE types in C

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76693/new/ https://reviews.llvm.org/D76693 ___

[PATCH] D76694: [Sema][SVE] Allow casting SVE types to themselves in C

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. Okay, then LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76694/new/ https://reviews.llvm.org/D76694 ___

[PATCH] D76528: [clang codegen] Clean up handling of vectors with trivial-auto-var-init.

This revision was automatically updated to reflect the committed changes. Closed by commit rG3f1defa6e2df: [clang codegen] Clean up handling of vectors with trivial-auto-var-init. (authored by efriedma). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D75661: Remove SequentialType from the type heirarchy.

efriedma updated this revision to Diff 252432. efriedma edited the summary of this revision. efriedma added a comment. Rebased. (CGDecl.cpp changes landed separately.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75661/new/ https://reviews.llvm.or

[PATCH] D76678: [SveEmitter] Add range checks for immediates and predicate patterns.

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7571 + else if (Builtin->LLVMIntrinsic != 0) { +llvm::Type* OverloadedTy = getSVEType(TypeFlags); + I'm not sure why you need a second way to convert SVE types separate from Convert

[PATCH] D76693: [Sema][SVE] Allow ?: to select between SVE types in C

efriedma added a comment. Do we want to allow stuff like `x ? (svint8_t)0 : (signed char)0` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76693/new/ https://reviews.llvm.org/D76693 ___ cfe-commits mail

[PATCH] D76694: [Sema][SVE] Allow casting SVE types to themselves in C

efriedma added a comment. Is it not legal to cast an SVE type to any type other than itself? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76694/new/ https://reviews.llvm.org/D76694 ___ cfe-commits mai

[PATCH] D76690: [AST][SVE] Treat built-in SVE types as POD

efriedma added inline comments. Comment at: clang/lib/AST/Type.cpp:2515 + if (BaseTy->isSizelessBuiltinType()) +return true; + Can you rearrange this so isSizelessBuiltinType() is at the bottom? It should be rare. (Assuming it doesn't need to be before th

[PATCH] D76689: [Sema][SVE] Fix handling of initialisers for built-in SVE types

efriedma added a comment. Semantically, this makes sense. I'm not really happy with the use of the term "scalar initializer" to refer to initializing a vector. Seems likely to confuse users. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76689/new

[PATCH] D76078: [AArch64][SVE] Add a pass for SVE intrinsic optimisations

efriedma added inline comments. Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:233 + bool Changed = false; + for (auto *F : Functions) { +DominatorTree *DT = &getAnalysis(*F).getDomTree(); Iterating over a SmallPtrSet is non-deterministic. In th

[PATCH] D76528: [clang codegen] Clean up handling of vectors with trivial-auto-var-init.

efriedma added a comment. Actually looking again, I'm not sure there's any way to make clang generate a bool vector at the moment. Added tests for the rest. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76528/new/ https://reviews.llvm.org/D76528

[PATCH] D76528: [clang codegen] Clean up handling of vectors with trivial-auto-var-init.

efriedma updated this revision to Diff 252169. efriedma edited the summary of this revision. efriedma added a comment. Add testcase to show missing vector handling. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76528/new/ https://reviews.llvm.org/D

[PATCH] D76528: [clang codegen] Clean up handling of vectors with trivial-auto-var-init.

efriedma added a comment. Not sure what sort of test would catch this; it's not crashing, and the types with the tail padding issue are not naturally exposed by target intrinsic headers (so it would only show up for code using ext_vector_type, or maybe OpenCL code). I only tripped over the iss

[PATCH] D76527: Don't export symbols from clang/opt/llc if plugins are disabled.

This revision was automatically updated to reflect the committed changes. Closed by commit rG896335bfb8ea: Don't export symbols from clang/opt/llc if plugins are disabled. (authored by efriedma). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76527/ne

[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

efriedma added a comment. I think we should remove the LangRef rule that says "sret" pointers have to be dereferenceable/naturally aligned, and let the frontend add explicit aligned/dereferenceable markings where appropriate. (At that point, sret has no target-independent meaning; it's just to

[PATCH] D76504: [clang] Fix crash during template sema checking

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/Sema/SemaChecking.cpp:1655 +// greater than 0. When `size` is value dependent we cannot evaluate its +// value so we bail<< out. +i

[PATCH] D76528: [clang codegen] Clean up handling of vectors with trivial-auto-var-init.

efriedma created this revision. efriedma added reviewers: glider, jfb. Herald added a subscriber: dexonsmith. Herald added a project: clang. The code was pretending to be doing something useful with vectors, but really it was doing nothing: the element type of a vector is always a scalar type, so

[PATCH] D76527: Don't export symbols from clang/opt/llc if plugins are disabled.

efriedma created this revision. efriedma added reviewers: MaskRay, serge-sans-paille, Meinersbur. Herald added subscribers: cfe-commits, dexonsmith, mgorny. Herald added a project: clang. The only reason we export symbols from these tools is to support plugins; if we don't have plugins, exporting

<    5   6   7   8   9   10   11   12   13   14   >