[PATCH] D75097: [cc1as] Unset UseNamesOnTempLabels

efriedma added a comment. Does UseNamesOnTempLabels really need to be configurable? If we're going to force it to false everywhere, might as well just get rid of it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75097/new/ https://reviews.llvm.or

[PATCH] D75097: [cc1as] Unset UseNamesOnTempLabels

efriedma added a comment. In terms of tests, we can add cc1as tests to clang/test/Misc; just need to be marked `REQUIRES: arm-registered-target` or something like that. That said, if we need to write cc1as-specific tests for anything other than the command-line parsing, probably something has g

[PATCH] D75097: [MC] Default MCContext::UseNamesOnTempLabels to false and only use it for MCAsmStreamer

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

[PATCH] D75370: [test] Fix flang test failures due to not handling .exe

efriedma added a comment. Do you have any idea why this isn't failing on the public buildbots? There are multiple Windows buildbots that are running these tests (for example, http://lab.llvm.org:8011/builders/llvm-clang-win-x-aarch64, and http://lab.llvm.org:8011/builders/llvm-clang-x86_64-exp

[PATCH] D75298: [Clang][SVE] Parse builtin type string for scalable vectors

efriedma added a comment. Please update the documentation comment at the top of Builtins.def . Can we avoid a gigantic, expensive to parse arm_sve.h somehow? Given that the compiler has to know the signatures for all these buitins anyway, can we just make including arm_sve.h set some flag in t

[PATCH] D75298: [Clang][SVE] Parse builtin type string for scalable vectors

efriedma added a comment. > The main reason we couldn't remove the header file is because of the > short-forms/overloaded intrinsics How are you planning to handle them with your current approach? Anyway, that isn't really relevant to the question of why we need to `#define svld1_u8(...) __bui

[PATCH] D75486: [SVE] Make CompositeType not inherit from Type

efriedma added a comment. It's possible to mess with the way "cast<>" actually casts values by specializing the function template in question. That, plus an appropriate classof implementations, should allow you to avoid the whole CompositeType::get thing. See, for example, https://github.com

[PATCH] D75572: [Sema][SVE] Reject sizeof and alignof for sizeless types

efriedma added a comment. The planned changes to RequireCompleteType should catch this, right? If we want a specialized diagnostic for sizeless types in RequireCompleteType, we should probably just pass it to RequireCompleteType. (Otherwise, you'll end up with the "wrong" diagnostic in templa

[PATCH] D75572: [Sema][SVE] Reject sizeof and alignof for sizeless types

efriedma added a comment. Just looked at the comment on D75572 ; not sure if RequireCompleteType is going to reject sizeless types. In any case, you have to instantiate templates using RequireCompleteType before you can determine whether a type is sizeless. R

[PATCH] D62962: Clang implementation of sizeless types

efriedma added a comment. Okay, thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62962/new/ https://reviews.llvm.org/D62962 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.ll

[PATCH] D75572: [Sema][SVE] Reject sizeof and alignof for sizeless types

efriedma added a comment. I think the specialized error messages are useful. I don't have a strong opinion on what order the patches should land. The difference between emitting the diagnostic in RequireCompleteType, vs. letting the caller do it, is basically just that it's harder to forget to

[PATCH] D75298: [Clang][SVE] Parse builtin type string for scalable vectors

efriedma added a comment. > This is because many of the builtins are protected by an architecture guard > like `#ifdef __ARM_FEATURE_SVE` or `#ifdef __ARM_FEATURE_SVE2`. I think TARGET_HEADER_BUILTIN has support for requiring target features. Granted, we don't use it much at the moment, so the

[PATCH] D75572: [Sema][SVE] Reject sizeof and alignof for sizeless types

efriedma added a reviewer: rsmith. efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Please give it a day or two before you merge in case someone else has an opinion on the new API. Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D75571: [Sema][SVE] Add tests for valid and invalid type usage

efriedma added inline comments. Comment at: clang/test/SemaCXX/sizeless-1.cpp:8 +// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++17 %s +// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only

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

efriedma created this revision. Herald added subscribers: cfe-commits, kerbowa, hiraditya, nhaehnle, jvesely, arsenm. Herald added projects: clang, LLVM. efriedma added a parent revision: D75660: Remove CompositeType class. This is a bit more dubious than removing CompositeType, given the number

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG98286b569d01: [Headers] Fix compatibility between arm_acle.h and intrin.h (authored by efriedma). Changed prior to commit: https://reviews.llvm.org/D69012?vs=225130&id=226999#toc Repository: rG LLVM

[PATCH] D67678: PR17164: Change clang's default behavior from -flax-vector-conversions=all to -flax-vector-conversions=integer.

efriedma added inline comments. Comment at: docs/ReleaseNotes.rst:84 + In a future release of Clang, we intend to change the default to + ``-fno-lax-vector-conversions``. + And if you want to allow your code to build with both clang-9 and clang-10, you have to

[PATCH] D67678: PR17164: Change clang's default behavior from -flax-vector-conversions=all to -flax-vector-conversions=integer.

efriedma added inline comments. Comment at: docs/ReleaseNotes.rst:84 + In a future release of Clang, we intend to change the default to + ``-fno-lax-vector-conversions``. + rsmith wrote: > efriedma wrote: > > And if you want to allow your code to build with bot

[PATCH] D67678: PR17164: Change clang's default behavior from -flax-vector-conversions=all to -flax-vector-conversions=integer.

efriedma added inline comments. Comment at: docs/ReleaseNotes.rst:84 + In a future release of Clang, we intend to change the default to + ``-fno-lax-vector-conversions``. + kristof.beyls wrote: > efriedma wrote: > > rsmith wrote: > > > efriedma wrote: > > > > A

[PATCH] D69618: NeonEmitter: clean up prototype modifiers

efriedma added a comment. It looks like this patch contains a few other changes, besides the changes to the prototypes. In particular, the change to CGBuiltin.cpp, and there are a few new lines in the .td files that don't correspond to anything in the old versions. Is that accidental, or is i

[PATCH] D69716: NeonEmitter: remove special 'a' modifier.

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

[PATCH] D69715: NeonEmitter: change Type representation. NFC.

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. I guess the extra checks are due to existing code "accidentally" doing the right thing? Have you verified this is NFC in terms of the generated arm_neon.h etc? LGTM Repository: rG LLV

[PATCH] D69869: [clang-tools-extra] fix the check for if '-latomic' is necessary

efriedma added a comment. > while failing on 8-bit atomic operations as there is no hardware support That's weird; it should be possible to emulate 8-bit atomic operations on top of 64-bit cmpxchg. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D698

[PATCH] D69925: remove redundant LLVM version from version string when setting CLANG_VENDOR

efriedma added a reviewer: rjmccall. efriedma added a comment. I think the reason it's written this way is that CLANG_VERSION_STRING might not correspond to an LLVM version? For example, Apple has its own versioning scheme. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D67409: [RISCV] enable LTO support, pass some options to linker.

efriedma added inline comments. Herald added a subscriber: sameer.abuasal. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:498 +CmdArgs.push_back( +Args.MakeArgString(Twine("-plugin-opt=-target-abi=") + ABIName)); } I don't think this change i

[PATCH] D53608: [builtins] Build float128 soft float builtins for x86_64.

efriedma added a comment. Yes, it makes sense to provide these routines, but someone has to write the code to make it work. This patch is currently incomplete: > Took another look and seems like long double is hardcoded in many of the > builtins. So I think the current patch needs to rename a

[PATCH] D69925: remove redundant LLVM version from version string when setting CLANG_VENDOR

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. If we expect that CLANG_VERSION_STRING and BACKEND_PACKAGE_STRING are going to be the same, then the extra note isn't really buying us anything. LGTM Repository: rG LLVM Github Monorep

[PATCH] D67409: [RISCV] enable LTO support, pass some options to linker.

efriedma added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:498 +CmdArgs.push_back( +Args.MakeArgString(Twine("-plugin-opt=-target-abi=") + ABIName)); } khchen wrote: > efriedma wrote: > > I don't think this change is right

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

efriedma added a comment. It wouldn't be that hard to add an equivalent flag to opt; just a matter of calling the API on TargetLibraryInfo. -disable-simplify-libcalls already exists, but I guess that's not quite what you want. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D70143: Check result of emitStrLen before passing it to CreateGEP

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

[PATCH] D68862: [ARM] Allocatable Global Register Variables for ARM

efriedma added a comment. Approval is not ever a time-based process; someone appropriate actually has to do the work of reviewing the patch. If a patch doesn't get reviewed, you "ping" it a couple times, to note that you're waiting for a review. If it still isn't reviewed at that point, and y

[PATCH] D69618: NeonEmitter: clean up prototype modifiers

efriedma added a comment. LGTM with a couple nits. Comment at: clang/include/clang/Basic/arm_neon_incl.td:203 // // The modifier 'd' means "default" and does not modify the base type in any // way. The available modifiers are given below. 'd' is gone.

[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies

efriedma added a comment. Could you write a description somewhere of what the overall region tree should look like for a switch? Comment at: clang/test/CoverageMapping/switch.cpp:32 switch (i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+6]]:2 = #4 -nop();

[PATCH] D69618: NeonEmitter: clean up prototype modifiers

efriedma added a comment. It looks like this broke vcreate_u16 and friends. From http://lab.llvm.org:8011/builders/aosp-O3-polly-before-vectorizer-unprofitable/builds/1224/steps/build-aosp/logs/stdio : external/skia/src/opts/SkBitmapProcState_filter_neon.h:53:42: error: C-style cast from sc

[PATCH] D70157: Align branches within 32-Byte boundary

efriedma added inline comments. Comment at: llvm/include/llvm/MC/MCFragment.h:684 + // need to be aligned. + static unsigned AlignBoundarySize; + // AlignMaxPrefixSize - The maximum size of prefixes per instruction. Global variables are forbidden in LLVM libra

[PATCH] D70253: [AArch64][SVE2] Implement remaining SVE2 floating-point intrinsics

efriedma added inline comments. Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:898 + llvm_i32_ty], +[IntrNoMem]>; + kmclaughlin wrote: > sdesmalen wrote: > > I'd expect the `llvm_i32_ty` to be an immediate for these instruct

[PATCH] D69471: [Coverage] Revise format to reduce binary size

efriedma added inline comments. Comment at: llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp:53-56 +Error E = +zlib::compress(FilenamesStr, CompressedStr, zlib::BestSizeCompression); +if (E) + llvm_unreachable("Unexpected failure in zlib::compress"); -

[PATCH] D71000: [AArch64][SVE] Implement intrinsics for non-temporal loads & stores

efriedma added inline comments. Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:6587 + else if (!Size) +Size = MemVT.getStoreSize().getKnownMinSize(); In order for alias analysis to correctly handle a MachineMemOperand, the "Size" of an operati

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

efriedma added a comment. It looks like this patch accidentally suppresses certain warnings when _FORTIFY_SOURCE is enabled. For example, Sema::CheckMemaccessArguments only gets called for recognized builtin functions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D71000: [AArch64][SVE] Implement intrinsics for non-temporal loads & stores

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:10800 +MINode->getOperand(3), DAG.getUNDEF(LoadVT), +

[PATCH] D71320: [IR] Split out target specific intrinsic enums into separate headers

efriedma added inline comments. Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:759 Type *Ty) const; - int getIntImmCost(Intrinsic::ID IID, unsigned Idx, const APInt &Imm, -Type *Ty) const; + int getIntImmCostIntrin(Intrinsi

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

efriedma added a comment. Can you update https://clang.llvm.org/docs/LanguageExtensions.html#langext-vectors with a description of the interaction between the different language features here? It's unfortunate that the gcc extension is explicitly incompatible with OpenCL, but I guess we're stu

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

efriedma added inline comments. Comment at: clang/lib/AST/Decl.cpp:3019 +if (SL.isValid()) + return SM.isInSystemHeader(SL); + } I'm a little concerned about this; we're subtly changing our generated code based on the "system-headerness" of the definit

[PATCH] D71167: [Driver] Default to -momit-leaf-frame-pointer for AArch64

efriedma added a comment. If I'm understanding correctly, this does nothing without D71168 ? And together with D71168 , the net change for clang is that "-mno-omit-leaf-frame-pointer" works correctly, but everything else stays t

[PATCH] D71167: [Driver] Default to -momit-leaf-frame-pointer for AArch64

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

[PATCH] D71556: [AArch64][SVE] Implement intrinsic for non-faulting loads

efriedma added a comment. I'm not sure it's legal to transform a non-faulting load to a load with a non-faulting flag? At least, we'd need to consider the implications of that very carefully. In particular, I'm concerned about the interaction with intrinsics that read/write FFR. I mean, you

[PATCH] D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO

efriedma added a comment. My primary concern with this is that I'm not sure you should be passing this information separately, as opposed to encoding it into the bitcode. On ARM, the ABI for a file is generally derived from the target triple. For example, "arm-eabi" is soft-float, "arm-eabihf"

[PATCH] D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO

efriedma added a comment. > Unfortunately on RISCV, the ABI info is only derived from -mabi option and > the target triple does not encode ABI info (same to gcc). How does gcc decide the default ABI for a target? Do you explicitly specify it at configure time? > example 1: > What's expected

[PATCH] D71556: [AArch64][SVE] Implement intrinsic for non-faulting loads

efriedma added a comment. > Perhaps we can clarify the intent that the non-faulting mode may have > side-effects by renaming the flag to something like > NonFaultingWithSideEffects? We could specify something like that... but that probably requires some changes to target-independent DAGCombine

[PATCH] D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO

efriedma added a comment. > But in RISCV clang emits the same IR for different ABI (-mabi), This is not true. For simple cases, it does, yes, but there are some weird edge cases for functions with many arguments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D71698: [AArch64][SVE] Add intrinsic for non-faulting loads

efriedma added inline comments. Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:5333 + // We need a layer of indirection because early machine code passes balk at + // physical register (i.e. FFR) uses that have no previous definition. + let hasSideEffects = 1, hasNoSch

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

efriedma added a comment. To be rigorous, we should perform "pointer" checking for every operation that performs pointer arithmetic. Then we should perform "lvalue" checking (which doesn't allow pointers one past the end) in the following places: 1. When we take the address of an lvalue. 2. Wh

[PATCH] D71698: [AArch64][SVE] Add intrinsic for non-faulting loads

efriedma added inline comments. Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:5333 + // We need a layer of indirection because early machine code passes balk at + // physical register (i.e. FFR) uses that have no previous definition. + let hasSideEffects = 1, hasNoSch

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

efriedma added a comment. > and it seems to involve a lot of AST traversal I was thinking we'd just call into SemaChecking in appropriate places. I guess there's a little AST traversal to figure whether an expression forms an array address. Your idea seems simpler. > remove elements from the

[PATCH] D75690: [SVE][Inline-Asm] Add constraints for SVE ACLE types

efriedma added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.h:95 +case 'U': // Three-character constraint; add "@3" hint for later parsing. + R = std::string("@3") + std::string(Constraint, 3); + Constraint += 2; Is "@3" some LLVM

[PATCH] D74386: [SVE] Update API ConstantVector::getSplat() to use ElementCount.

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4497 Value *CodeGenFunction::EmitNeonSplat(Value *V, Constant *C) { - unsigned nElts = V->getType()->getVectorNumElements(); - Value* SV = llvm::ConstantVector::getSplat(nElts, C); + auto EC = V->getT

[PATCH] D75570: [AST][SVE] Add new Type queries for sizeless 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/D75570/new/ https://reviews.llvm.org/D75570 ___

[PATCH] D75298: [Clang][SVE] Parse builtin type string for scalable vectors

efriedma added a comment. Changing the way we expose the builtins isn't going to affect most of the code related to the SVE intrinsics. I'm fine sticking with a known working approach, and trying to address that issue later. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75298/new/ ht

[PATCH] D75690: [SVE][Inline-Asm] Add constraints for SVE ACLE 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/D75690/new/ https://reviews.llvm.org/D75690 ___ cfe-commits mailing list cfe-commi

[PATCH] D75571: [Sema][SVE] Add tests for valid and invalid type usage

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

[PATCH] D75738: [Sema][SVE] Reject by-copy capture of sizeless 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/D75738/new/ https://reviews.llvm.org/D75738 ___

[PATCH] D75734: [Sema][SVE] Reject atomic sizeless 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/D75734/new/ https://reviews.llvm.org/D75734 ___

[PATCH] D75737: [Sema][SVE] Don't allow fields to have sizeless type

efriedma added inline comments. Comment at: clang/lib/Sema/SemaLambda.cpp:1633 +if (RequireCompleteSizedType(Loc, FieldType, + diag::err_field_incomplete_or_sizeless)) { RD->setInvalidDecl(); Can BuildCaptureField actual

[PATCH] D75573: [Sema][SVE] Reject aligned/_Alignas for sizeless types

efriedma added a comment. LGTM Technically, you could still do math on the address, and the difference would be visible. Granted, it's unlikely to matter in practice. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75573/new/ https://reviews.llvm.o

[PATCH] D75736: [Sema][SVE] Don't allow static or thread-local variables to have sizeless type

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

[PATCH] D76088: [Sema][SVE] Don't allow sizeless objects to be thrown

efriedma added a comment. Why is throwing an `svint8_t *` an error? I don't see any obvious reason to forbid that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76088/new/ https://reviews.llvm.org/D76088 ___

[PATCH] D76082: [Sema][SVE] Reject arrays of sizeless types

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. It might be reasonable to support this at some point, but sure, I'm fine rejecting it for now. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D76086: [Sema][SVE] Reject arithmetic on pointers to sizeless types

efriedma added a comment. Do we really want to forbid this? The expected semantics are clear, and the obvious lowering to getelementptr just works. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76086/new/ https://reviews.llvm.org/D76086 __

[PATCH] D76084: [Sema][SVE] Reject subscripts on pointers to sizeless types

efriedma added a comment. I'm not sure we actually want to reject this; let's discuss on the review for D76086 , since subscripting is sort of a special case of arithmetic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D74386: [SVE] Update API ConstantVector::getSplat() to use ElementCount.

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

[PATCH] D75737: [Sema][SVE] Don't allow fields to have sizeless type

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/Sema/SemaLambda.cpp:1633 +if (RequireCompleteSizedType(Loc, FieldType, + diag::err_field_incomplete_or_size

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

efriedma added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64TargetMachine.cpp:441 + // Expand any SVE vector library calls that we can't code generate directly. + bool ExpandToOptimize = (TM->getOptLevel() != CodeGenOpt::None); + if (EnableSVEIntrinsicOpts && TM

[PATCH] D75298: [Clang][SVE] Parse builtin type string for scalable vectors

efriedma added inline comments. Comment at: clang/utils/TableGen/SveEmitter.cpp:86 + OS << "#ifndef __cplusplus\n"; + OS << "#include \n"; + OS << "#endif\n\n"; I'd prefer to avoid depending on stdbool if it isn't necessary. Comment at: cla

[PATCH] D76088: [Sema][SVE] Don't allow sizeless objects to be thrown

efriedma added a comment. We already have a special case for void* here; it's trivial to extend that to also check for sizeless types. I don't want to add weird semantic restrictions just to save a few characters in the compiler. I agree that most people won't run into it either way. Reposito

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

efriedma added a comment. I would like to kill off all the code you're modifying, and let ExprConstant be the final arbiter of whether a constant is in fact constant. But that's probably a larger project than you really want to mess with. The big missing piece of that is that we currently don

[PATCH] D76088: [Sema][SVE] Don't allow sizeless objects to be thrown

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

[PATCH] D76087: [Sema][SVE] Reject sizeless types in exception specs

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM In theory, we actually could allow throwing sizeless types; as far as I know, nothing in the ABI actually blocks it. But probably not worth the work to implement it. Repository:

[PATCH] D76086: [Sema][SVE] Reject arithmetic on pointers to sizeless types

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM People are going to want to iterate over arrays in memory to write simple intrinsic loops. If we don't allow `p + i` to work, people will be forced to write `(__SVInt8_t*)((char*)p

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

efriedma added inline comments. Comment at: clang/lib/AST/Expr.cpp:3164 + const QualType &QT = cast(this)->getDecl()->getType(); + if (QT->isStructureType() && QT.isConstQualified()) +return true; nickdesaulniers wrote: > efriedma wrote: > > nic

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

efriedma added a comment. I think the code that disables constant evaluation for C is just https://github.com/llvm/llvm-project/blob/dcaf13a4048df3dad55f1a28cde7cefc99ccc057/clang/lib/AST/ExprConstant.cpp#L13918 and https://github.com/llvm/llvm-project/blob/dcaf13a4048df3dad55f1a28cde7cefc99ccc

[PATCH] D76090: [Sema][SVE] Don't allow sizeless types to be caught

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

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

efriedma added a comment. > Removing those two LangOpt checks isn't enough for the test cases to run. Removing those two checks should unblock const-eval for structs in general. I wasn't thinking about whether there something else for global variables specifically, though. It looks like there

[PATCH] D75298: [Clang][SVE] Parse builtin type string for scalable vectors

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:86 + OS << "#ifndef __cplusplus\n"; + OS << "#include \n"; + OS << "#endif\n\n"; sdesmalen wrote

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

efriedma added a comment. > Isn't that what my patch is doing? (Codegen walking the AST/InitListExpr, > generating Constants)? Yes. The issue is we we don't really want to duplicate the logic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76096/ne

[PATCH] D76169: [WIP][AST] Allow ExprConstant to evaluate structs in C.

efriedma created this revision. efriedma added a reviewer: nickdesaulniers. Herald added a project: clang. Just a proof of concept to show this works. Handles all the examples from D76096 , as far as I can tell. Repository: rG LLVM Github Monorepo https://re

[PATCH] D76218: [Sema][SVE] Reject "new" with sizeless 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/D76218/new/ https://reviews.llvm.org/D76218 ___

[PATCH] D76086: [Sema][SVE] Reject arithmetic on pointers to sizeless types

efriedma added a comment. I don't think there's really any reason to force the user into this sort of thing... but again, we can relax this later, so I'm not that concerned about it at the moment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7608

[PATCH] D76084: [Sema][SVE] Reject subscripts on pointers to sizeless types

efriedma added a comment. We already support sizeof expressions where the result can't be computed at compile-time. For example: `int f(int n, int (*p)[n]) { return sizeof(*p); }`. (For constant contexts specifically, see HandleSizeof() in ExprConstant.cpp.) I think any argument that it woul

[PATCH] D76084: [Sema][SVE] Reject subscripts on pointers to sizeless types

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. This patch LGTM, at least for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76084/new/ https://reviews.llvm.org/D76084 _

[PATCH] D76219: [Sema][SVE] Reject "delete" with sizeless 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/D76219/new/ https://reviews.llvm.org/D76219 ___

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013 + if (V->hasInit()) +return Visit(V->getInit(), V->getType()); +return nullptr; rsmith wrote: > nickdesaulniers wrote: > > efriedma wrote: > > > You need to be

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013 + if (V->hasInit()) +return Visit(V->getInit(), V->getType()); +return nullptr; rsmith wrote: > efriedma wrote: > > rsmith wrote: > > > nickdesaulniers wrote: >

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

efriedma created this revision. efriedma added reviewers: nhaehnle, t.p.northover, dblaikie. Herald added a reviewer: bollu. Herald added subscribers: cfe-commits, jfb. Herald added a project: clang. Removes deprecated overloads of LoadInst constructor, CallInst::Create, InvokeInst::Create, IRBui

[PATCH] D75660: Remove CompositeType class

efriedma updated this revision to Diff 250909. efriedma added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Add one missing change in clang unittests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75660/new/ https

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

efriedma updated this revision to Diff 250910. efriedma added a comment. Rebase. Fix a test failure. A few other minor tweaks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75661/new/ https://reviews.llvm.org/D75661 Files: clang/lib/CodeGen/CG

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

efriedma marked an inline comment as done. efriedma added a comment. I looked at getting rid of getSequentialNumElements/getSequentialElementType, but I'm not sure that would actually make the code more clear. I avoided them in cases where they clearly weren't helpful. Comme

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

efriedma updated this revision to Diff 250928. efriedma added a comment. Minor fix to AMDGPU changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75661/new/ https://reviews.llvm.org/D75661 Files: clang/lib/CodeGen/CGDecl.cpp clang/lib/CodeGe

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

efriedma added a comment. I guess the API names could be a bit confusing, yes. Any suggestions for better names? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75661/new/ https://reviews.llvm.org/D75661 _

[PATCH] D75660: Remove CompositeType class

This revision was automatically updated to reflect the committed changes. Closed by commit rGe24e95fe900a: Remove CompositeType class. (authored by efriedma). Changed prior to commit: https://reviews.llvm.org/D75660?vs=250909&id=251170#toc Repository: rG LLVM Github Monorepo CHANGES SINCE L

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

efriedma updated this revision to Diff 251218. efriedma added a comment. I went through the exercise of completely removing the notion of a "sequential" type; the result comes out something like this. Broadly, the places that might be able to use some notion of "sequential" mostly fall into two

<    9   10   11   12   13   14   15   16   17   18   >