[PATCH] D46071: Representing the target device information in the LLVM IR

efriedma added a comment. I agree this needs an RFC; I don't understand how you plan to use this information. Repository: rOMP OpenMP https://reviews.llvm.org/D46071 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D46112: Allow _Atomic to be specified on incomplete types

efriedma added a comment. This generally makes sense. Need some tests to make sure we emit an appropriate error if you try to actually use atomic operators (load/store) or intrinsics (__atomic_is_lock_free etc.) with an incomplete type. And a test that code generation emits something appropri

[PATCH] D46109: [ARM, AArch64] Add intrinsics for dot product instructions

efriedma added inline comments. Comment at: test/CodeGen/arm-neon-dot-product.c:1 +// RUN: %clang_cc1 -triple armv8-linux-gnueabihf -target-cpu cortex-a57 -target-feature +dotprod \ +// RUN: -disable-O0-optnone -emit-llvm -o - %s | opt -S -instcombine | FileCheck %s --

[PATCH] D46155: Add warning flag -Wordered-compare-function-pointers.

efriedma created this revision. efriedma added a reviewer: rsmith. The C standard doesn't allow comparisons like "f1 < f2" (where f1 and f2 are function pointers), but we allow them as an extension. Add awarning flag to control this warning. (Not sure I like the name, but this seems to des

[PATCH] D46109: [ARM, AArch64] Add intrinsics for dot product instructions

efriedma added inline comments. Comment at: test/CodeGen/arm-neon-dot-product.c:1 +// RUN: %clang_cc1 -triple armv8-linux-gnueabihf -target-cpu cortex-a57 -target-feature +dotprod \ +// RUN: -disable-O0-optnone -emit-llvm -o - %s | opt -S -instcombine | FileCheck %s --

[PATCH] D46013: [ARM] Conform to AAPCS when passing overaligned composites as arguments

efriedma added a comment. I'd like to see some tests for __attribute((packed)). Comment at: lib/CodeGen/TargetInfo.cpp:5787 + } else { + TyAlign = getContext().getTypeAlign(Ty) ; + } Whitespace. Repository: rC Clang https://reviews.llvm.org/D46013

[PATCH] D46328: [X86] Mark all x86 specific builtins as nothrow.

efriedma added a comment. You could write a test like `static_assert(noexcept(__builtin_ia32_bzhi_di(0,0)), "");`. I don't think it has any other effect. Repository: rC Clang https://reviews.llvm.org/D46328 ___ cfe-commits mailing list cfe-com

[PATCH] D46328: [X86] Mark all x86 specific builtins as nothrow.

efriedma added a comment. We don't expect user code to call these builtins directly anyway, so probably not useful to test. Repository: rC Clang https://reviews.llvm.org/D46328 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists

[PATCH] D46328: [X86] Mark all x86 specific builtins as nothrow.

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D46328 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D46155: Add warning flag -Wordered-compare-function-pointers.

This revision was automatically updated to reflect the committed changes. Closed by commit rC331570: Add warning flag -Wordered-compare-function-pointers. (authored by efriedma, committed by ). Repository: rC Clang https://reviews.llvm.org/D46155 Files: include/clang/Basic/DiagnosticSemaKin

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

efriedma added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:2512 + if (D && isa(D) && + cast(D)->isCopyOrMoveConstructor()) +F->addParamAttr(1, llvm::Attribute::NoAlias); Why does it matter whether it's a copy constructor? The standard te

[PATCH] D45712: Diagnose invalid cv-qualifiers for friend decls.

efriedma updated this revision to Diff 154471. efriedma retitled this revision from "[WIP] Diagnose invalid cv-qualifiers for friend decls." to "Diagnose invalid cv-qualifiers for friend decls.". efriedma edited the summary of this revision. efriedma added a comment. Updated to handle the templat

[PATCH] D45712: Diagnose invalid cv-qualifiers for friend decls.

efriedma added a comment. I thought the template case wasn't using the same codepath, but apparently I was just confusing myself. The exact form "template friend class Y;" triggers an early out in Sema::ActOnFriendTypeDecl, so my code wasn't getting hit. I'll revise accordingly. Repository:

[PATCH] D45712: Diagnose invalid cv-qualifiers for friend decls.

efriedma updated this revision to Diff 154707. efriedma edited the summary of this revision. efriedma added a comment. Move declspec checks before call to GetTypeForDeclarator, since it can fail for class templates. Repository: rC Clang https://reviews.llvm.org/D45712 Files: lib/Sema/Sema

[PATCH] D49209: CodeGen: specify alignment for automatic variable initialization

efriedma added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:902 +Builder.CreateAlignedStore( +Init, Loc, Loc->getPointerAlignment(CGM.getDataLayout()), isVolatile); return; llvm::Value::getPointerAlignment is not the right way to compute t

[PATCH] D49209: CodeGen: specify alignment for automatic variable initialization

efriedma added inline comments. Comment at: lib/CodeGen/CGBuilder.h:260 +CharUnits::fromQuantity(Offset.getSExtValue(; + } + Not sure about the new helper. We already have CreateStructGEP and CreateConstArrayGEP which do approxi

[PATCH] D49209: CodeGen: specify alignment for automatic variable initialization

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/CodeGen/CGBuilder.h:260 +CharUnits::fromQuantity(Offset.getSExtValue(; + } + jfb wrote: > efriedma

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

efriedma added a comment. Adding startFrontendTimer/stopFrontendTimer helps a little, but it's still difficult to match a given startFrontendTimer to the corresponding stopFrontendTimer because they're in completely different functions in some cases. Do they really need to be scattered like th

[PATCH] D49330: [compiler-rt] Include -lm when using compiler-rt, due to dependencies in some __div methods.

efriedma added a comment. Probably compiler-rt should be fixed so it doesn't need libm, rather than fixing clang to add -lm. (All the functions it currently uses can be implemented with simple bit manipulation.) Repository: rC Clang https://reviews.llvm.org/D49330 __

[PATCH] D49424: [PowerPC] Handle __builtin_xxpermdi the same way as GCC does

efriedma added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:10780 // endian order so the shuffle mask must be adjusted for this on little // endian platforms (i.e. index is complemented and source vector reversed). +unsigned ElemIdx0 = (Index & 2) >> 1;;

[PATCH] D45712: Diagnose invalid cv-qualifiers for friend decls.

efriedma added a comment. Ping Repository: rC Clang https://reviews.llvm.org/D45712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.

efriedma added a comment. Are you sure this will actually do what you want, in general? I suspect it will end up missing bounds checks in some cases because it's running it too early (before mem2reg/inlining/etc). Repository: rC Clang https://reviews.llvm.org/D49492

[PATCH] D49508: [CodeGen] VisitMaterializeTemporaryExpr(): don't skip NoOp Casts.

efriedma added a comment. skipRValueSubobjectAdjustments has to match the rules in `[class.temporary]` in the standard, which includes skipping over certain explicit casts. Would it be enough to accumulate the skipped casts into a SmallVector, like we do for the skipped comma operators?

[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.

efriedma added a comment. This sanitizer has a bit of a strange design compared to other sanitizers; it tries to compute the size of the base object using the IR at the point the pass runs. So the later it runs, the more information it has. Trivial example: static int accumulate(int* foo, i

[PATCH] D68715: [mangle] Fix mangling where an extra mangle context is required.

efriedma added a comment. The refactoring of the getCurrentMangleNumberContext API mixed with functional changes makes this much harder to review quickly. Please separate it out. Comment at: clang/lib/Sema/SemaLambda.cpp:353 + Mangling

[PATCH] D68715: [mangle] Fix mangling where an extra mangle context is required.

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

[PATCH] D68720: Support -fstack-clash-protection for x86

efriedma added a comment. Is there some reason this isn't using the existing stack-probe-size attribute? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 ___ cfe

[PATCH] D68720: Support -fstack-clash-protection for x86

efriedma added a comment. Sorry, I meant the "probe-stack" attribute. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D68743: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 2.

efriedma created this revision. efriedma added reviewers: rsmith, SjoerdMeijer. Herald added a subscriber: kristof.beyls. Herald added a project: clang. Just running -fsyntax-only over arm_neon.h doesn't cover some intrinsics which are defined using macros. Add more test coverage for that, then

[PATCH] D68720: Support -fstack-clash-protection for x86

efriedma added a comment. > (b) is an issue, as pointed out in https://lwn.net/Articles/726587/ (grep for > valgrind) : from valgrind point of view, accessing un-allocated stack memory > triggers error, and we probably want to please valgrind > > Doing the call *after* the stack allocation is a

[PATCH] D68743: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 2.

efriedma updated this revision to Diff 224229. efriedma edited the summary of this revision. efriedma added a comment. Found another broken case. Fix, and add more test coverage. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68743/new/ https://reviews.llvm.org/D

[PATCH] D68743: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 2.

This revision was automatically updated to reflect the committed changes. Closed by commit rG30a96d3fcb76: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 2. (authored by efriedma). Changed prior to commit: https://reviews.llvm.org/D68743?vs=224229&id=224432#toc Repository: rG

[PATCH] D68838: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 3

efriedma created this revision. efriedma added reviewers: rsmith, SjoerdMeijer. Herald added a subscriber: kristof.beyls. Herald added a project: clang. It's completely impossible to check that I've actually found all the issues, due to the use of macros in arm_neon.h, but hopefully this time it'

[PATCH] D68720: Support -fstack-clash-protection for x86

efriedma added inline comments. Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:423 + AbsOffset - CurrentAbsOffset + PageSize); +if (FreeProbeIterator != MBB.end()) { + NumFrameFreeProbe++; Each probe has to hav

[PATCH] D68838: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 3

efriedma updated this revision to Diff 224496. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68838/new/ https://reviews.llvm.org/D68838 Files: test/CodeGen/aarch64-v8.2a-neon-intrinsics.c utils/TableGen/NeonEmitter.cpp Index: utils/TableGen/NeonEmitter.cpp =

[PATCH] D68849: [Parse] Don't speculatively parse an identifier in the wrong context.

efriedma created this revision. efriedma added a reviewer: rsmith. Herald added a project: clang. When we see something like `ihttps://bugs.llvm.org/show_bug.cgi?id=43080 (but not the issues currently marked as "duplicate"; I haven't dug deeply into the causes of those issues). Repository: rC

[PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

efriedma added a comment. Hmm, the ObjC changes were simpler than I expected. And you managed to avoid making changes to overload sets. The changes related to IsAddressOfOperand are a nice simplification. Would it make sense to always use ClassifyName from the parser, instead of using ActOnId

[PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM > I'm inclined to defer doing that for now, if that's OK :) Sure. Comment at: lib/Sema/SemaDecl.cpp:1191 + return NameClassification::ContextIndependentExpr( +

[PATCH] D68882: [test] Fix test failure

This revision was automatically updated to reflect the committed changes. Closed by commit rG4498d41932c6: [test] Fix test failure (authored by efriedma). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68882/new/ https://reviews.llvm.org/D68882 Files

[PATCH] D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls

efriedma added a comment. > Bug2: test case provided above. Please Lewis take a look at how this case can > be fixed. Isn't the issue just that the code is checking for PseudoTAIL, and not PseudoTAILIndirect? What happens if a tail call has more than 8 arguments? What happens for a must

[PATCH] D68838: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 3

efriedma added a comment. Ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68838/new/ https://reviews.llvm.org/D68838 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/l

[PATCH] D69012: [Headers] Fix compatibility between arm_acle.h and intrin.h

efriedma created this revision. efriedma added reviewers: rnk, chill. Herald added a subscriber: kristof.beyls. Herald added a project: clang. Make sure they don't both define __nop. We use a similar pattern for other functions in arm_acle.h. Repository: rC Clang https://reviews.llvm.org/D6

[PATCH] D68838: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 3

This revision was automatically updated to reflect the committed changes. Closed by commit rG5b0e039a7a7d: [ARM] Fix arm_neon.h with -flax-vector-conversions=none, part 3 (authored by efriedma). Changed prior to commit: https://reviews.llvm.org/D68838?vs=224496&id=225534#toc Repository: rG L

[PATCH] D69297: [ARM][AArch64] Implement __arm_rsrf, __arm_rsrf64, __arm_wsrf & __arm_wsrf64

efriedma added a comment. We have __builtin_bit_cast now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69297/new/ https://reviews.llvm.org/D69297 ___ cfe-commits mailing list cfe-commits@lists.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] 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] 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] 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. 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. 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 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] 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] 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. 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] 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] 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. 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] D83553: [PATCH 3/4][Sema][AArch64] Add codegen for arm_sve_vector_bits attribute

efriedma added inline comments. Comment at: clang/lib/CodeGen/CodeGenTypes.h:138 + llvm::Type *ConvertTypeForMem(QualType T, bool ForBitField = false, +bool EnforceFixedLengthSVEAttribute = false); c-rhodes wrote: > efriedma wro

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

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:152 + Align, Name, + /*ArraySize=*/nullptr, Alloca); Do we need to bitcast the result of CreateTempAlloca to a pointer to the array type? I'm concerned that we might miss a bit

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

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:152 + Align, Name, + /*ArraySize=*/nullptr, Alloca); c-rhodes wrote: > efriedma wrote: > > Do we need to bitcast the result of CreateTempAlloca to a pointer to the > > array typ

[PATCH] D84703: [clang codegen][AArch64] Use llvm.aarch64.neon.fcvtzs/u where it's necessary

efriedma created this revision. efriedma added reviewers: sdesmalen, SjoerdMeijer, t.p.northover, cameron.mcinally. Herald added subscribers: cfe-commits, danielkiss, kristof.beyls. Herald added a project: clang. fptosi/fptoui have similar, but not identical, semantics. In particular, the behav

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

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