[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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] D79587: [CodeGen][SVE] Legalisation of extends with scalable types

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/D79587/new/ https://reviews.llvm.org/D79587 ___ cfe-commits mailing list cfe-commi

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

efriedma added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:660 + /// with [[no_unique_attr]] Empty CXXRD invovled in the CXXRD. + int IsRealFirstMember; + Please explain here what the different values (-1, 0, 1, 2) mean. Or use an enum.

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

efriedma added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:666 +FirstNonOverlappingEmptyFieldHandled + } FirstNonOverlappingEmptyFieldStatus; + Instead of specifically tracking whether you've found an OverlappingEmpty field, could you

[PATCH] D81311: [RFC] LangRef: Define inmem parameter attribute

efriedma added a comment. On a normal, stack-based machine, there are two ways to pass an aggregate to a function: directly, or indirectly. Directly means something like "byval" or "preallocated": the value of the pointer is not a value the caller can control, but instead refers to some fixed

[PATCH] D81311: [RFC] LangRef: Define inmem parameter attribute

efriedma added a comment. I think the important bit here is that we need to distinguish between the C function types `void f(A)` and `void f(A*)`. If they're both lowered to pointers with the same type, we need an attribute to distinguish them. CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D81408: [builtins] Improve compatibility with 16 bit targets

efriedma added inline comments. Comment at: compiler-rt/lib/builtins/fp_lib.h:49 -static __inline int rep_clz(rep_t a) { return __builtin_clz(a); } +static __inline int rep_clz(rep_t a) { return clzsi(a); } MaskRay wrote: > atrosinenko wrote: > > MaskRay wrot

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

efriedma added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:11571 + Diag(Loc, + getLangOpts().C11 + ? diag::ext_typecheck_compare_complete_incomplete_pointers I think this condition is backwards? Should be `!g

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

efriedma added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:666 +FirstNonOverlappingEmptyFieldHandled + } FirstNonOverlappingEmptyFieldStatus; + Xiangling_L wrote: > efriedma wrote: > > Instead of specifically tracking whether you've fo

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

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/D79719/new/ https://reviews.llvm.org/D79719 ___

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

efriedma added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:11571 + Diag(Loc, + getLangOpts().C11 + ? diag::ext_typecheck_compare_complete_incomplete_pointers rsmith wrote: > pestctrl wrote: > > efriedma wrote:

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

efriedma added inline comments. Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:524 + CallConv)) +return; EVT ValueVT = Val.getValueType(); I'm not sure I understand why the standard getCopyFromParts/g

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

efriedma added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6451 + "%1 is %select{|in}3complete">, + InGroup; def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn< `InGroup` Comment at: clang/l

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

efriedma added inline comments. Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:524 + CallConv)) +return; EVT ValueVT = Val.getValueType(); pratlucas wrote: > efriedma wrote: > > I'm not sure I underst

[PATCH] D79155: [CodeGen] Increase applicability of ffine-grained-bitfield-accesses for targets with limited native integer widths

efriedma added a comment. Please add a comment explaining what OffsetInRecord means; then LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79155/new/ https://reviews.llvm.org/D79155 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

efriedma added a reviewer: aqjune. efriedma added a comment. Herald added a subscriber: wuzish. I usually like to start reading this sort of patch with the proposed LangRef change, but I'm not seeing one. There are a couple of related issues here in the existing representation of IR: 1. The way

[PATCH] D81721: [SVE] Ensure proper mangling of ACLE tuple types

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/D81721/new/ https://reviews.llvm.org/D81721 ___

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

efriedma added a comment. In D81678#2089041 , @aqjune wrote: > > @efriedma > > The way that call argument coercion works is unsound in the presence of > > poison. An integer can't be partially poisoned: it's either poison, or not > > poison. We probabl

[PATCH] D80712: [SVE] Add checks for no warnings in SVE tests

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/D80712/new/ https://reviews.llvm.org/D80712 ___ cfe-commits mailing list cfe-commi

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

efriedma added a comment. In terms of the C++ API, we definitely want to provide an API phrased positively in terms of individual arguments, so transforms don't have to deal with inverted logic. In terms of the actual internal memory representation, or textual IR, maybe we can be a bit more fl

[PATCH] D73543: [clang] Add support for __builtin_memcpy_inline

efriedma added inline comments. Comment at: clang/test/Sema/builtins-memcpy-inline.c:7 +#warning defined as expected +// expected-warning@-1 {{defined as expected}} +#endif melver wrote: > It appears that the expected-warning check here is guarded by the #if as

[PATCH] D79167: [SVE][CodeGen] Legalisation of vsetcc with scalable types

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. Is it possible to write tests for this that don't result in a "max" or "min" operation? Or does that fail for some other reason? Otherwise LGTM. CHANGES SINCE LAST ACTION https://revi

[PATCH] D81463: [SveEmitter] Add builtins for tuple creation (svcreate2/svcreate3/etc)

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

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM I'll merge for you; how do you want to be credited on the "Author" line of the commit message? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

efriedma added a comment. So I guess we've discussed the following alternatives so far: 1. Attach the "frozen" attribute everywhere; this makes the textual IR generated by clang messy, and likely bloats memory usage (not sure by how much). 2. Invert the meaning of the attribute; this makes reaso

[PATCH] D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute

efriedma added a comment. I'm tempted to say this is a bugfix for the implementation of no_unique_address, and just fix it globally for all ABIs. We're never going to get anything done here if we require a separate patch for each ABI variant clang supports. Comment at: clan

[PATCH] D82085: [TRE] allow TRE for non-capturing calls.

efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:94 /// If it contains any dynamic allocas, returns false. static bool canTRE(Function &F) { // Because of PR962, we don't TRE dynamic allocas. If we're not go

[PATCH] D82085: [TRE] allow TRE for non-capturing calls.

efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:94 /// If it contains any dynamic allocas, returns false. static bool canTRE(Function &F) { // Because of PR962, we don't TRE dynamic allocas. avl wrote: > ef

[PATCH] D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute

efriedma added a comment. In D81583#2139002 , @uweigand wrote: > In D81583#2137277 , @efriedma wrote: > > > I'm tempted to say this is a bugfix for the implementation of > > no_unique_address, and just fix it globa

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

efriedma added a comment. Please also add testcases with select constant expressions, to test constant folding. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83360/new/ https://reviews.llvm.org/D83360 ___

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

efriedma added a comment. > Should we remove the handling from llvm::ConstantFoldSelectInstruction It seems silly to remove the handling from InstSimplify, but not constant folding. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83360/new/ https:/

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

efriedma added a comment. In D83360#2140224 , @regehr wrote: > @craig.topper ok, I agree that should work. alive doesn't like it -- is this > an alive bug @nlopes? a freeze should not yield undef. > https://alive2.llvm.org/ce/z/mWAsYv Did you mean to c

[PATCH] D82085: [TRE] allow TRE for non-capturing calls.

efriedma added a comment. I think I'd like to see a testcase where there are multiple recursive calls, but only one is a tail call that can be eliminated. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:474 + // Operand Bundles or not marked as TailCall. +

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

efriedma added a subscriber: tgt. efriedma added a comment. > that's fine but I still don't understand why the counterexample to my version > says %x2 in @src can be undef If I'm understanding correctly, this reduces to something like the following: define i32 @src() { %x2 = freeze i32 undef

[PATCH] D82085: [TRE] allow TRE for non-capturing calls.

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/D82085/new/ https://reviews.llvm.org/D82085 ___

[PATCH] D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute

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/D81583/new/ https://reviews.llvm.org/D81583 ___ cfe-commits mailing list cfe-commi

[PATCH] D83553: [PATCH 3/4][Sema][AArch64] Add codegen for arm_sve_vector_bits attribute

efriedma added a comment. What's the tradeoff of representing these in IR as vscale'ed vector types, as opposed to fixed-wdith vector types? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83553/new/ https://reviews.llvm.org/D83553 ___ cfe-c

[PATCH] D83553: [PATCH 3/4][Sema][AArch64] Add codegen for arm_sve_vector_bits attribute

efriedma added a comment. > If you mean alloca's for single vectors I was really referring to the IR values themselves, not the memory representation. Since the width of the vectors is known, you could emit IR without any mention of scalable types at all (assuming the backend was extended to

[PATCH] D83550: [PATCH 1/4][Sema][AArch64] Add parsing support for arm_sve_vector_bits attribute

efriedma added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7784 + // The __ARM_FEATURE_SVE_BITS macro must be defined when using this attribute. + auto &PP = S.getPreprocessor(); + if (!PP.isMacroDefined("__ARM_FEATURE_SVE_BITS")) { c-rhodes wrot

[PATCH] D83553: [PATCH 3/4][Sema][AArch64] Add codegen for arm_sve_vector_bits attribute

efriedma added a comment. In D83553#2151591 , @sdesmalen wrote: > In D83553#2148429 , @efriedma wrote: > > > > If you mean alloca's for single vectors > > > > I was really referring to the IR values themselves, not

[PATCH] D81466: [SveEmitter] Add builtins for struct loads/stores (ld2/ld3/etc)

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7890 + Value *Predicate = EmitSVEPredicateCast(Ops[0], VTy); + Value *BasePtr= Builder.CreateBitCast(Ops[1], VecPtrTy);

[PATCH] D82085: [TRE] markTails marks call sites as tailcalls though some of them are not.

efriedma added a comment. All "tail" needs to mean is "this call does not reference the current function's stack." That's all, no more or less. The relevant documentation and APIs are a bit confusing. A "tail" marking is a prerequisite for tailcall optimization, but it's not really related to

[PATCH] D81408: [builtins] Improve compatibility with 16 bit targets

efriedma added a comment. > Those flaky test failures seems to be due to ld.lld being not built from > source as part of testing compiler-rt/-only patches. That should be something we can fix in the build system. compiler-rt/test/CMakeLists.txt has a list of executables which the tests depend

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

This revision was automatically updated to reflect the committed changes. Closed by commit rGc310bf8256f8: [Sema] Comparison of pointers to complete and incomplete types (authored by pestctrl, committed by efriedma). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D80977: Diagnose cases where the name of a class member is used within a class definition before the member name is declared.

efriedma added a comment. (Partial review; I'll continue reviewing later.) Comment at: clang/lib/Sema/SemaDecl.cpp:1543 +// in class 'C', where we look up 'f' to determine if we're declaring a +// constructor. + } else if (D->isInIdentifierNamespace(Lookup.FirstIDNS))

[PATCH] D79167: [SVE][CodeGen] Legalisation of vsetcc with scalable types

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

[PATCH] D81408: [builtins] Improve compatibility with 16 bit targets

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM We don't need to block this on the pre-merge checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81408/new/ https://reviews.llvm.org

[PATCH] D82085: [TRE] allow TRE for non-capturing calls.

efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:130 +IsNocapture = true; + else if (Function *CalledFunction = CB.getCalledFunction()) { +if (CalledFunction->getBasicBlockList().size() > 0 && ---

[PATCH] D82085: [TRE] allow TRE for non-capturing calls.

efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:825 + // The local stack holds all alloca instructions and all byval arguments. + AllocaDerivedValueTracker Tracker; + for (Argument &Arg : F.args()) { avl wrot

[PATCH] D82085: [TRE] allow TRE for non-capturing calls.

efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:825 + // The local stack holds all alloca instructions and all byval arguments. + AllocaDerivedValueTracker Tracker; + for (Argument &Arg : F.args()) { avl wrot

[PATCH] D82085: [TRE] allow TRE for non-capturing calls.

efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:825 + // The local stack holds all alloca instructions and all byval arguments. + AllocaDerivedValueTracker Tracker; + for (Argument &Arg : F.args()) { avl wrot

[PATCH] D82085: [TRE] allow TRE for non-capturing calls.

efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:825 + // The local stack holds all alloca instructions and all byval arguments. + AllocaDerivedValueTracker Tracker; + for (Argument &Arg : F.args()) { avl wrot

[PATCH] D81285: [builtins] Change si_int to int in some helper declarations

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM. (Please wait a few days before merging to see if anyone else has comments.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81285/new/

[PATCH] D82085: [TRE] allow TRE for non-capturing calls.

efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:825 + // The local stack holds all alloca instructions and all byval arguments. + AllocaDerivedValueTracker Tracker; + for (Argument &Arg : F.args()) { avl wrot

[PATCH] D82085: [TRE] allow TRE for non-capturing calls.

efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:825 + // The local stack holds all alloca instructions and all byval arguments. + AllocaDerivedValueTracker Tracker; + for (Argument &Arg : F.args()) { avl wrot

[PATCH] D81678: Introduce frozen attribute at call sites for stricter poison analysis

efriedma added a comment. In D81678#2109059 , @nlopes wrote: > I'm a bit concerned with this patch as it increases the amount of UB that > LLVM exploits without any study of the impact. > For example, right now it's ok do this with clang (not with consta

[PATCH] D79052: [clang codegen] Fix alignment of "Address" for incomplete array pointer.

This revision was automatically updated to reflect the committed changes. Closed by commit rGbf8b63ed296c: [clang codegen] Fix alignment of "Address" for incomplete array pointer. (authored by efriedma). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D81282: [builtins] Move more float128-related helpers to GENERIC_TF_SOURCES list

efriedma added inline comments. Comment at: compiler-rt/lib/builtins/CMakeLists.txt:142 powisf2.c powitf2.c subdf3.c Missed powitf? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81282/new/ https://reviews

[PATCH] D81282: [builtins] Move more float128-related helpers to GENERIC_TF_SOURCES list

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/D81282/new/ https://reviews.llvm.org/D81282 ___

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

efriedma added a comment. In D81678#2120709 , @guiand wrote: > Could anyone point me to where I might need to make a change for that? LibCallSimplifier::optimizePow Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D81728: [InstCombine] Add target-specific inst combining

efriedma added a comment. > This combines instructions, so I think it belongs into the InstCombine pass. > On the other hand, the f16 form of the intrinsics is not available on all > targets, so this combination cannot be applied unconditionally but it needs > to be gated depending on the targe

[PATCH] D80952: [FPEnv][Clang][Driver] Disable constrained floating point on targets lacking support.

efriedma added reviewers: john.brawn, nikic. efriedma added subscribers: nikic, john.brawn. efriedma added a comment. I'm not sure what the status is of constrained fp on arm/aarch64; some patches went in, but I'm not sure what sort of testing was done. @john.brawn @nikic ? Otherwise, I think t

[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] 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] 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] 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] 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] 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] 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] 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] 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] 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 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] 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] 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] 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] D78748: [SveEmitter] Add builtins for svld1rq

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/D78748/new/ https://reviews.llvm.org/D78748 ___

[PATCH] D78751: [SveEmitter] Add builtins for scatter stores

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/D78751/new/ https://reviews.llvm.org/D78751 ___

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

efriedma accepted this revision. efriedma added a comment. LGTM Comment at: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_prfb.c:22 + // expected-error@+1 {{argument value -1 is outside the valid range [0, 13]}} + return svprfb(pg, base, -1); +} ---

[PATCH] D77597: [SveEmitter] Add IsAppendSVALL and builtins for svptrue and svcnt[bhwd]

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/D77597/new/ https://reviews.llvm.org/D77597 ___ cfe-commits mailing list cfe-commi

[PATCH] D78812: [SVE][CodeGen] Fix legalisation for scalable types

efriedma added inline comments. Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:738 + *DAG.getContext(), IntermediateVT.getScalarType(), DestVectorNoElts, + ValueVT.isScalableVector()); if (ValueVT != BuiltVectorTy) { Can we use Ele

[PATCH] D78853: [Sema] Fix null pointer dereference warnings [1/n]

efriedma added a comment. Please don't add null checks for pointers that can't be null. It makes the code slower and harder to understand. And least one of the checks you added is actively breaking the code. In some cases, the analysis is pointing to cases where the code could be made more c

[PATCH] D78755: [SveEmitter] Add builtins for svlen

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/D78755/new/ https://reviews.llvm.org/D78755 ___

[PATCH] D78958: [SVE] Update EmitSVEPredicateCast to take a ScalableVectorType

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/D78958/new/ https://reviews.llvm.org/D78958 ___

[PATCH] D78960: [SveEmitter] Add builtins for ternary ops (fmla, fmad, etc)

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM For patches that only touch arm_sve.td, I trust that you've written it correctly; if there were any mistakes, it would be very hard to catch them in review anyway. However you prefe

<    1   2   3   4   5   6   7   8   9   10   >