[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] 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] 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 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] 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 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] 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] 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] 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(); 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/lib/Sema/SemaExpr.cpp:11571 + Diag(Loc, + getLangOpts().C11 + ? diag::ext_typecheck_compare_complete_incomplete_pointers rsmith wrote: > pestctrl wrote: > > efriedma wrote:

[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] 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] 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] 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] 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] 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] 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] 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] 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] D79721: [Clang][AArch64] Capturing proper pointer alignment for Neon vld1 intrinsicts

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

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

efriedma updated this revision to Diff 267999. efriedma added a comment. This should work correctly, now, I think? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79052/new/ https://reviews.llvm.org/D79052 Files: clang/lib/CodeGen/CodeGenModule.cp

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

efriedma updated this revision to Diff 267994. efriedma added a comment. "Address" the review comments. Not really happy with this, but not sure what else to do. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79052/new/ https://reviews.llvm.org/D7

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

efriedma planned changes to this revision. efriedma added a comment. Hang on, I submitted this too early. Need to look a bit more. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79052/new/ https://reviews.llvm.org/D79052

[PATCH] D80828: [Clang][A32/T32][Linux] -O1 implies -fomit-frame-pointer

efriedma accepted this revision. efriedma added a comment. LGTM. For non-Android, I think it makes sense to align with gcc as much as possible. > This is mostly > for legacy/compatibility reasons, and the 2019 Q4 revision of the ARM > AAPCS looks to standardize r11 as the frame pointer for aa

[PATCH] D79587: [CodeGen][SVE] Legalisation of extends with scalable types

efriedma added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:10695 +// Truncate to prevent a DUP with an over wide constant +SDValue Trunc = DAG.getNode(ISD::TRUNCATE, DL, EltTy, Dup->getOperand(0)); + It's not legal to gene

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

efriedma added a comment. The problem for the command-line arguments in particular is that they aren't really new; clang has been eating them for a long time, without any warning. So if -frounding-math crashes the compiler, that's a regression. Repository: rG LLVM Github Monorepo CHANGES S

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

efriedma added a comment. > Clang :: Misc/warning-flags.c Like the file says, all warnings should be controlled by a -W flag. If you're going to add a new warning, you need a corresponding flag. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80952

[PATCH] D79587: [CodeGen][SVE] Legalisation of extends with scalable types

efriedma added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:10676 +ConstantSDNode *C = dyn_cast(Dup->getOperand(0)); +uint64_t ExtVal = C->getZExtValue(); + kmclaughlin wrote: > efriedma wrote: > > Do you need to truncate E

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

efriedma added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6447 + InGroup; +def warn_typecheck_compare_complete_incomplete_pointers : ExtWarn< + "ordered comparison of complete and incomplete pointers (%0 and %1)">, pestctrl wr

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

efriedma added a comment. Okay. Thinking about it a bit more, I think I'd prefer using a pattern where we redirect stderr to a file, and CHECK it separately, to make sure we don't miss a warning because it got interleaved into the middle of stdout. Repository: rG LLVM Github Monorepo CHANG

[PATCH] D79587: [CodeGen][SVE] Legalisation of extends with scalable types

efriedma added inline comments. Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:4337 + // Collect the (promoted) operands + SDValue Ops[] = { GetPromotedInteger(InOp0), BaseIdx }; + In general, there are four possibilities for legaliz

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

efriedma added a comment. I guess we could do this as a temporary measure, if you think it's useful; eventually, of course, the codepath to print the warning will go away. Since you checked this, how many tests do still print warnings? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D79721: [Clang][AArch64] Capturing proper pointer alignment for Neon vld1 intrinsicts

efriedma added a comment. I'm not completely happy with using EmitPointerWithAlignment here... but I guess it's the same thing we do for 32-bit ARM, so it must be mostly usable in practice. I'd like to see some tests in clang/test/CodeGen/aarch64-neon-intrinsics.c showing what happens if you p

[PATCH] D79721: [Clang][AArch64] Capturing proper pointer alignment for Neon vld1 intrinsicts

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:10331 +auto Alignment = CGM.getNaturalPointeeTypeAlignment( +E->getArg(0)->IgnoreParenCasts()->getType()); Ops[0] = Builder.CreateBitCast(Ops[0], llvm::PointerType::getUnqual(VTy)); -

[PATCH] D79721: [Clang][AArch64] Capturing proper pointer alignment for Neon vld1 intrinsicts

efriedma added a comment. This caused https://bugs.llvm.org/show_bug.cgi?id=46084 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79721/new/ https://reviews.llvm.org/D79721 ___ cfe-commits mailing list c

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

efriedma added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6446 + "ordered comparison of complete and incomplete pointers (%0 and %1)">, + InGroup; +def warn_typecheck_compare_complete_incomplete_pointers : ExtWarn< ext_typeche

[PATCH] D47092: downgrade strong type info names to weak_odr linkage

efriedma added a comment. Oh, you're right, sorry, the suggestion was to make asan instrumentation introduce the alias, not the frontend. In which case you'd want that fix plus D46665 , I think: external linkage optimizes better. Repository: rC Clang CHANG

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

efriedma added a comment. Any thoughts on "this diagnostic needs to be restricted to C99 or earlier"? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79945/new/ https://reviews.llvm.org/D79945 ___ cfe-co

[PATCH] D47092: downgrade strong type info names to weak_odr linkage

efriedma added a comment. I think the preferred solution is something like https://bugs.llvm.org/show_bug.cgi?id=37545#c4, which is slightly different from both this patch and D46665 . Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org

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

efriedma added a comment. Oh, wait, I was looking at the C11 version. This diagnostic needs to be restricted to C99 or earlier, and only to relational comparisons, I think. And given it isn't really a valuable diagnostic, I think it should be off by default (an "Extension" diagnostic). Repos

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

efriedma added a comment. I'm not sure how you derive this requirement from the standard; the section in question doesn't use the words "complete" or "incomplete" at all. Am I missing something obvious? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

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

efriedma added inline comments. Comment at: clang/include/clang/AST/RecordLayout.h:74 + /// The maximum allowed field alignment. This is set by #pragma pack. + CharUnits MaxFieldAlignment; + jasonliu wrote: > efriedma wrote: > > If we have to keep around extra

[PATCH] D80166: [CGCall] Annotate reference parameters with "align" attribute.

efriedma marked 2 inline comments as done. efriedma added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2116 +if (PTy->isObjectType()) { + if (unsigned Alignment = getContext().getTypeAlignIfKnown(PTy)) +RetAttrs.addAlignmentAttr( rjm

[PATCH] D80046: [StackSafety] Make full LTO to attach metadata if MTE is enabled

efriedma added a comment. Okay, I think we're mostly on the same page, then. I have a few issues here: 1. Whether the backend wants this information is really a per-function decision, not a per-module decision; using module-level metadata is sort of weird. 2. Having weird rules like this makes

[PATCH] D79877: [clang][SveEmitter] SVE builtins for `svusdot` and `svsudot` ACLE.

efriedma accepted this revision. efriedma added inline comments. Comment at: clang/include/clang/Basic/arm_sve.td:1249 +def SVSUDOT_S: SInst<"svsudot[_s32]","ddqb", "i", MergeNone, "aarch64_sve_usdot", [ReverseUSDOT]>; +def SVSUDOT_N_S : SInst<"svsudot[_n_s32]",

[PATCH] D80046: [StackSafety] Make full LTO to attach metadata if MTE is enabled

efriedma added a comment. If I'm understanding this correctly, the stack-safe metadata on allocas is both produced and consumed at LTO-time. So at the point where the stack-safe metadata would be produced, we can compute whether any later passes will query it. Given that, why do you need a mo

[PATCH] D79914: [CodeGen][NFC] Fix test/CodeGen/pr45476.cpp to specify target triple.

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

[PATCH] D79877: [clang][SveEmitter] SVE builtins for `svusdot` and `svsudot` ACLE.

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM with one small comment. Comment at: clang/include/clang/Basic/arm_sve.td:1249 +def SVSUDOT_S: SInst<"svsudot[_s32]","ddqb", "i", MergeNone, "aarch64_s

[PATCH] D79357: [SveEmitter] Add builtins for svdup and svindex

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

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

efriedma added a comment. Sure, I'm happy with the second option. > Alignment at the call site might be higher than of the copy, breaking with > the idea that the call site and callee "properties" match. Though, the > attributes can probably be kept in sync if we teach the relevant parts. The

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

efriedma added a comment. This approach seems to reflect the consensus from the mailing list. Comment at: clang/include/clang/AST/RecordLayout.h:74 + /// The maximum allowed field alignment. This is set by #pragma pack. + CharUnits MaxFieldAlignment; + If we

[PATCH] D79587: [CodeGen][SVE] Legalisation of extends with scalable types

efriedma added a comment. For sunpckhi... no, not really. You'd need to either add a new opcode, or add a new shuffle operation of some sort. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79587/new/ https://reviews.llvm.org/D79587

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

efriedma added a comment. > @efriedma I'm a bit confused. Could you propose some wording so I get a > feeling where you want to go? Depending on which direction we go, either: - "Attributes on a function or callsite describe the behavior of the callee excluding the implied copy. For example,

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

efriedma added a comment. > Do you object to say that the call site argument and the argument point to > distinct memory locations or something else? Like I said, my issue is with the "Attributes on the call site argument and function argument are associated with the original and copied memory

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

efriedma added inline comments. Comment at: llvm/docs/LangRef.rst:1050 +the call site argument and function argument are associated with the +original and copied memory respectively. The copy is considered to be local +memory of the callee. That means, a callee can wr

[PATCH] D79588: [llvm][Support] Use std::atomic for llvm::call_once

efriedma requested changes to this revision. efriedma added a comment. This revision now requires changes to proceed. This code is basically untested; I'd rather not touch it until we eventually kill it off. (See the definition of LLVM_THREADING_USE_STD_CALL_ONCE.) Repository: rG LLVM Github

[PATCH] D79579: [SveEmitter] Add builtins for svmovlb and svmovlt

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

[PATCH] D79584: [SVE] Add a couple of extra sizeless type tests

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

[PATCH] D79587: [CodeGen][SVE] Legalisation of extends with scalable types

efriedma added a comment. Can you explain why target-independent legalization isn't suitable here? I'd prefer not to have target-specific type legalization for every operation on vscale'ed types. (The target-independent name of AArch64ISD::SUNPKLO is ISD::SIGN_EXTEND_VECTOR_INREG.) Reposito

[PATCH] D79054: [NFC] Improve doc string to mention that paths in diff are used as-is

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

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

efriedma accepted this revision. efriedma added a comment. LGTM with one minor comment Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5607 + // * Register R12(IP), + // * Condition codes (and thus the CPSR register) + // yroux wrote: > efriedma wrote:

[PATCH] D78812: [SVE][CodeGen] Fix legalisation for 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/D78812/new/ https://reviews.llvm.org/D78812 ___ cfe-commits mailing list cfe-commi

[PATCH] D79478: [CodeGen][SVE] Lowering of shift operations with scalable 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/D79478/new/ https://reviews.llvm.org/D79478 ___

[PATCH] D79480: [SveEmitter] Add builtins for SVE2 Polynomial arithmetic

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/lib/CodeGen/CGBuiltin.cpp:7813 + llvm::ScalableVectorType *Ty = getSVEType(TypeFlags); + return Builder.CreateBitCast(Call

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

efriedma added inline comments. Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5813 + // Don't touch the link register + if (MI.readsRegister(ARM::LR, TRI) || MI.modifiesRegister(ARM::LR, TRI)) +return outliner::InstrType::Illegal; yroux wrote: > efri

[PATCH] D79357: [SveEmitter] Add builtins for svdup and svindex

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:8047 +Value *PFalse = Constant::getNullValue(PTrue->getType()); +Value *Sel = Builder.CreateSelect(CmpNE, PTrue, PFalse); +return EmitSVEPredicateCast(Sel, cast(Ty)); sdesmale

[PATCH] D79244: [Sema] Put existing warning under -Wexcess-initializers

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

[PATCH] D79244: [Sema] Put existing warning under -Wexcess-initializers

efriedma added a comment. Should we stick ext_excess_initializers_in_char_array_initializer and ext_initializer_string_for_char_array_too_long in the same warning group? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79244/new/ https://reviews.llvm

[PATCH] D79054: [NFC] Improve documentation for -i and update example git one liner

efriedma added a comment. We should probably encourage people to use git-clang-format with git repositories. It naturally doesn't have this sort of fragility because it integrates with the repository more tightly. Comment at: clang/tools/clang-format/clang-format-diff.py:41

[PATCH] D79357: [SveEmitter] Add builtins for svdup and svindex

efriedma added inline comments. Comment at: clang/include/clang/Basic/arm_sve.td:1050 def SVCOMPACT: SInst<"svcompact[_{d}]", "dPd", "ilUiUlfd", MergeNone, "aarch64_sve_compact">; -// SVDUP_LANE(to land in D78750) +def SVDUP_LANE : SInst<"svdup_lane[_{d}]",

[PATCH] D78756: [SveEmitter] Add builtins for svreinterpret

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

[PATCH] D78756: [SveEmitter] Add builtins for svreinterpret

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7880 +return Builder.CreateBitCast(Val, Ty); + } + efriedma wrote: > sdesmalen wrote: > > efriedma wrote: > > > sdesmalen wrote: > > > > efriedma wrote: > > > > > I'm vaguely suspici

[PATCH] D78750: [SveEmitter] Add builtins for svdupq and svdupq_lane

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

[PATCH] D79087: [SVE][Codegen] Lower legal min & max operations

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

[PATCH] D76689: [Sema][SVE] Fix handling of initialisers for built-in SVE 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/D76689/new/ https://reviews.llvm.org/D76689 ___

[PATCH] D78750: [SveEmitter] Add builtins for svdupq and svdupq_lane

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:8054 + +Value *Alloca = Builder.CreateAlloca(EltTy, Builder.getInt32(NumOpnds)); +for (unsigned I = 0; I < NumOpnds; ++I) Please use something like `CreateTempAlloca(llvm::ArrayTy

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

efriedma added inline comments. Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5806 +if (Opc == ARM::BL || Opc == ARM::tBL || Opc == ARM::BLX || +Opc == ARM::tBLXr || Opc == ARM::tBLXi) + UnknownCallOutlineType = outliner::InstrType::LegalTerminator; --

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

efriedma added inline comments. Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5607 + // * Register R12(IP), + // * Condition codes (and thus the CPSR register) + // If you control all the instructions that execute, you don't need to worry about what th

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

efriedma added a comment. The -ffine-grained-bitfield-accesses seems to generate weird results in certain cases. For example, on x86 we generate an unaligned load in the following example. struct S { long c : 8; long z: 24; long : 0; }; struct S s; int f() { return s.c+s.z; } I guess t

[PATCH] D79087: [SVE][Codegen] Lower legal min & max operations

efriedma added inline comments. Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:3851 + def : SVE_1_Op_Imm_Arith_Pred_Pat(NAME # _S)>; + def : SVE_1_Op_Imm_Arith_Pred_Pat(NAME # _D)>; } kmclaughlin wrote: > efriedma wrote: > > I don't see any test for th

[PATCH] D78756: [SveEmitter] Add builtins for svreinterpret

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7880 +return Builder.CreateBitCast(Val, Ty); + } + sdesmalen wrote: > efriedma wrote: > > sdesmalen wrote: > > > efriedma wrote: > > > > I'm vaguely suspicious this might be wrong fo

[PATCH] D79087: [SVE][Codegen] Lower legal min & max operations

efriedma added a subscriber: huihuiz. efriedma added inline comments. Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:3851 + def : SVE_1_Op_Imm_Arith_Pred_Pat(NAME # _S)>; + def : SVE_1_Op_Imm_Arith_Pred_Pat(NAME # _D)>; } I don't see any test for this

[PATCH] D78756: [SveEmitter] Add builtins for svreinterpret

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7880 +return Builder.CreateBitCast(Val, Ty); + } + sdesmalen wrote: > efriedma wrote: > > I'm vaguely suspicious this might be wrong for big-endian targets. I mean, > > this isn't

[PATCH] D78756: [SveEmitter] Add builtins for svreinterpret

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7880 +return Builder.CreateBitCast(Val, Ty); + } + I'm vaguely suspicious this might be wrong for big-endian targets. I mean, this isn't unreasonable, but users might be surprised

[PATCH] D78677: [SveEmitter] Add builtins for gather prefetches

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/lib/CodeGen/CGBuiltin.cpp:7739 + +// Index needs to be passed as scaled offset. +llvm::Type *MemEltTy = SVEBuiltinMe

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

efriedma marked an inline comment as done. efriedma added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:176 CharUnits Alignment; - if (T->isIncompleteType()) { + if (T->getBaseElementTypeUnsafe()->isIncompleteType()) { Alignment = CharUnits::One();

[PATCH] D78750: [SveEmitter] Add builtins for svdupq and svdupq_lane

efriedma added a comment. Is there some reason you decided to implement these particular functions as inline functions directly in the header? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78750/new/ https://reviews.llvm.org/D78750

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

efriedma created this revision. efriedma added a reviewer: rjmccall. Herald added a project: clang. rsmith added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:176 CharUnits Alignment; - if (T->isIncompleteType()) { + if (T->getBaseElementTypeUnsafe()->is

[PATCH] D79039: [SveEmitter] Add builtins for svmov_b and svnot_b.

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

[PATCH] D78842: [SVE][NFC] Use ScalableVectorType in CGBuiltin

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

[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

[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] 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] 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] 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] 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] 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] 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] 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 ___

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