[PATCH] D90329: [PowerPC] Fix va_arg in Objective-C on 32-bit ELF targets

2020-10-29 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4723 + bool isInt = Ty->isIntegerType() || Ty->hasPointerRepresentation() || + Ty->isAggregateType(); bool isF64 = Ty->isFloatingType() && getContext().getTypeSize(Ty) == 64;

[PATCH] D89573: [Driver] Incorporate -mfloat-abi in the computed triple on ARM

2020-10-20 Thread Eli Friedman via Phabricator via cfe-commits
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/D89573/new/ https://reviews.llvm.org/D89573 ___ cfe-commits mailing list

[PATCH] D89573: [Driver] Incorporate -mfloat-abi in the computed triple on ARM

2020-10-16 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:807 +default: + break; +} Do we want to error in the "default" case? Not that anyone is likely to use -mfloat-abi on those targets, but I'd prefer not to silently

[PATCH] D89523: PR44406: Follow behavior of array bound constant folding in more recent versions of GCC.

2020-10-16 Thread Eli Friedman via Phabricator via cfe-commits
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/D89523/new/ https://reviews.llvm.org/D89523

[PATCH] D89523: PR44406: Follow behavior of array bound constant folding in more recent versions of GCC.

2020-10-15 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:2278 ArraySize, , Diagnoser, - (S.LangOpts.GNUMode || S.LangOpts.OpenCL) ? Sema::AllowFold -: Sema::NoFold); + S.LangOpts.OpenCL ?

[PATCH] D89523: PR44406: Follow behavior of array bound constant folding in more recent versions of GCC.

2020-10-15 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. Also update the documentation? See https://clang.llvm.org/docs/UsersManual.html#differences-between-various-standard-modes . Comment at: clang/lib/Sema/SemaDecl.cpp:5939 +ElemTy = TryToFixInvalidVariablyModifiedType(ElemTy, Context, +

[PATCH] D89362: [MS] Apply `inreg` to AArch64 sret parms on instance methods

2020-10-15 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. Makes a lot more sense now. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89362/new/ https://reviews.llvm.org/D89362

[PATCH] D89362: [MS] Apply `inreg` to AArch64 sret parms on instance methods

2020-10-13 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/CodeGen/MicrosoftCXXABI.cpp:1077 -static bool hasMicrosoftABIRestrictions(const CXXRecordDecl *RD) { +static bool isCXX14Aggregate(const CXXRecordDecl *RD) { // For AArch64, we use the C++14 definition of an aggregate,

[PATCH] D89130: [WIP][Sparc] Fix long double on 32-bit Solaris/SPARC

2020-10-09 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. SparcTargetLowering::LowerCall_32 is Sparc-specific code. Maybe you can take a look at where it's crashing, and ask some more specific question? The compiler-rt requirement for __uint128_t is a bit inconvenient, I guess; maybe it could be changed, but that's probably

[PATCH] D86447: [AST] Change return type of getTypeInfoInChars to a proper struct instead of std::pair.

2020-10-07 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM. Sorry I forgot about this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86447/new/ https://reviews.llvm.org/D86447

[PATCH] D88789: [InstCombine] Revert rL226781 "Teach InstCombine to canonicalize loads which are only ever stored to always use a legal integer type if one is available." (PR47592)

2020-10-05 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. > Keeping loads and stores of pointers as pointers to the extent possible > doesn't seem like a bad idea, but I'm worried people will feel like this > gives a *semantic* guarantee that isn't really there. Fundamentally, LLVM > still doesn't currently have typed

[PATCH] D88013: [AArch64] Correct parameter type for unsigned Neon scalar shift intrinsics

2020-10-05 Thread Eli Friedman via Phabricator via cfe-commits
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/D88013/new/ https://reviews.llvm.org/D88013

[PATCH] D88789: [InstCombine] Revert rL226781 "Teach InstCombine to canonicalize loads which are only ever stored to always use a legal integer type if one is available." (PR47592)

2020-10-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. In D88789#2310606 , @chandlerc wrote: > FWIW, I still very much feel that this is the correct canonicalization, and > that downstream problems *must* be fixed downstream. Avoiding this > canonicalization doesn't actually fix

[PATCH] D87962: [clang] Change the multi-character character constants from extension to implementation-defined.

2020-10-01 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. > Or should we change the four character case to "Remark" so it wouldn't be > warned even with the "-pandemic" option? There seems no other choice. There's a DefaultIgnore modifier for warnings that turns then off by default. We use this sparingly, since it's hard to

[PATCH] D86790: [FE] Use preferred alignment instead of ABI alignment for complete object when applicable

2020-09-23 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4539 CharUnits CCAlign = getParamTypeAlignment(Ty); CharUnits TyAlign = getContext().getTypeAlignInChars(Ty); jyknight wrote: > Xiangling_L wrote: > > jasonliu wrote: > > >

[PATCH] D88009: [AArch64] Fix return type of Neon scalar comparison intrinsics

2020-09-21 Thread Eli Friedman via Phabricator via cfe-commits
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/D88009/new/ https://reviews.llvm.org/D88009

[PATCH] D85118: [clang][AArch64] Correct return type of Neon vqmovun intrinsics

2020-09-17 Thread Eli Friedman via Phabricator via cfe-commits
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/D85118/new/ https://reviews.llvm.org/D85118

[PATCH] D87607: [clang][aarch64] Support implicit casts between GNU and SVE vectors

2020-09-16 Thread Eli Friedman via Phabricator via cfe-commits
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/D87607/new/ https://reviews.llvm.org/D87607 ___ cfe-commits mailing list

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

2020-09-16 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87615/new/ https://reviews.llvm.org/D87615 ___ cfe-commits mailing list

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

2020-09-15 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. Also, it would be nice to have some regression test coverage; add a Solaris RUN line to llvm/test/CodeGen/X86/stack-align2.ll ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87615/new/ https://reviews.llvm.org/D87615

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

2020-09-15 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: llvm/lib/Target/X86/X86Subtarget.cpp:270 + else if (isTargetSolaris() && !In64BitMode) +stackAlignment = Align(4); stackAlignment is initialized to 4 in the header, so `stackAlignment = Align(4)` here is a

[PATCH] D87607: [clang][aarch64] Support implicit casts between GNU and SVE vectors

2020-09-15 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:8522 + return VT->getElementType().getCanonicalType() == + getBuiltinVectorTypeInfo(BT).ElementType; } c-rhodes wrote: > efriedma wrote: > > We allow casting

[PATCH] D87607: [clang][aarch64] Support implicit casts between GNU and SVE vectors

2020-09-14 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:8522 + return VT->getElementType().getCanonicalType() == + getBuiltinVectorTypeInfo(BT).ElementType; } We allow casting SVE fixed-width vectors only if the

[PATCH] D87615: [clang][Driver] Force stack realignment on 32-bit Solaris/x86

2020-09-14 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. In D87615#2271297 , @ro wrote: > In D87615#2271268 , @RKSimon wrote: > >> Would it be possible to add some tests? > > Probably not reliably: the tests that usually fail bye the hundreds

[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

2020-09-10 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86621/new/ https://reviews.llvm.org/D86621 ___ cfe-commits mailing list

[PATCH] D87358: [clang][aarch64] Fix ILP32 ABI for arm_sve_vector_bits

2020-09-10 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/AST/Type.cpp:2324 // scalable and fixed-length vectors. -return Ctx.UnsignedCharTy; - case BuiltinType::SveInt16: -return

[PATCH] D87358: [clang][aarch64] Fix ILP32 ABI for arm_sve_vector_bits

2020-09-09 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/AST/Type.cpp:2339 case BuiltinType::SveInt32: -return Ctx.IntTy; +return IsILP32 ? Ctx.LongTy : Ctx.IntTy; case BuiltinType::SveUint32: sdesmalen wrote: > Rather than comparing with a specific

[PATCH] D87218: [builtins] Inline __paritysi2 into __paritydi2 and inline __paritydi2 into __parityti2.

2020-09-07 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87218/new/ https://reviews.llvm.org/D87218 ___ cfe-commits mailing list

[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

2020-09-02 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/Basic/Targets/Sparc.cpp:224 +Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8"); + } } ro wrote: > efriedma wrote: > > ro wrote: > > > efriedma wrote: > > > > ro wrote: > > > > > efriedma wrote:

[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

2020-09-02 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/Basic/Targets/Sparc.cpp:224 +Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8"); + } } ro wrote: > efriedma wrote: > > ro wrote: > > > efriedma wrote: > > > > ro wrote: > > > > > efriedma wrote:

[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

2020-09-01 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:350 return A->getValue(); +if (T.getArch() == llvm::Triple::sparc && T.isOSSolaris()) + return "v9"; ro wrote: > brad wrote: > > ro wrote: > > > efriedma

[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

2020-09-01 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/Basic/Targets/Sparc.cpp:224 +Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8"); + } } ro wrote: > efriedma wrote: > > ro wrote: > > > efriedma wrote: > > > > This probably should be refactored

[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

2020-09-01 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/Basic/Targets/Sparc.cpp:224 +Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8"); + } } ro wrote: > efriedma wrote: > > This probably should be refactored so the target-independent code generates

[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

2020-08-31 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. > While the fix proper is trivial: just two lines in > lib/Driver/ToolChains/CommonArgs.cpp, finding the right place has been > nightmarishly difficult: I'd have expected handling of a Solaris/SPARC CPU > default in either of Solaris or SPARC specific files, but not

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

2020-08-27 Thread Eli Friedman via Phabricator via cfe-commits
efriedma closed this revision. efriedma added a comment. It was reverted, then re-landed. It's in trunk now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 ___

[PATCH] D86101: [AArch64][SVE] Add missing debug info for ACLE types.

2020-08-26 Thread Eli Friedman via Phabricator via cfe-commits
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/D86101/new/ https://reviews.llvm.org/D86101 ___ cfe-commits mailing list

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

2020-08-21 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. As far as I know, there are basically three categories of things that depend on the alignment of a type. 1. The default alignment of load/store/alloca. On trunk, load/store/alloca always have explicitly specified alignment in memory. That said, old bitcode doesn't

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

2020-08-21 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. I'm afraid the AutoUpgrade component of this isn't compatible with existing IR without some additional work. I'm most concerned about cases like the following: #pragma pack(8) struct X { __int128 x; }; // Not a packed struct in IR because the native alignment is

[PATCH] D86187: [X86] Add support 'tune' in target attribute

2020-08-19 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86187/new/ https://reviews.llvm.org/D86187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

2020-08-19 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1847 +IsIncompleteArrayType ? CharUnits::Zero() : TI.first; +AlignIsRequired =

[PATCH] D86187: [X86] Add support 'tune' in target attribute

2020-08-19 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/test/Sema/attr-target.c:26 +//expected-warning@+1 {{unsupported tune 'hiss' in the 'target' attribute string; 'target' attribute ignored}} +int __attribute__((target("tune=hiss,tune=woof"))) apple_tree() { return 4; } +

[PATCH] D86065: [SVE] Make ElementCount members private

2020-08-18 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. > Perhaps now would be a good time to combine TypeSize and ElementCount into a > single Polynomial type? We don't have to implement the whole abstraction of > c*x^n (since we currently don't use the exponent, and don't distinguish > between X's) but if it's ever

[PATCH] D86101: [AArch64][SVE] Add missing debug info for ACLE types.

2020-08-18 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:734 + if (Info.ElementType == CGM.getContext().BoolTy) +NumElemsPerVG /= 8; + I guess NumElemsPerVG of a bool vector is always divisible by 8 because we don't expose ``

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

2020-08-18 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. I'm still concerned your approach to the computation of getTypeSize() is a ticking time bomb, but I'll take the cleanup even if the underlying motivation doesn't really make sense. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1847 +

[PATCH] D84992: [clang codegen] Use IR "align" attribute for static array arguments.

2020-08-18 Thread Eli Friedman via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG673dbe1b5eef: [clang codegen] Use IR align attribute for static array arguments. (authored by efriedma). Repository: rG LLVM Github Monorepo

[PATCH] D85384: [X86] Add basic support for -mtune command line option in clang

2020-08-18 Thread Eli Friedman via Phabricator via cfe-commits
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/test/Misc/target-invalid-cpu-note.c:49 + +// RUN: not %clang_cc1 -triple x86_64--- -tune-cpu not-a-cpu -fsyntax-only %s

[PATCH] D85743: [CodeGen][AArch64] Support arm_sve_vector_bits attribute

2020-08-14 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Like I mentioned on the review for the prototype, I still think we should try to implement a scheme that makes CK_BItCast between fixed and scalable types trivial. Doing coercion

[PATCH] D85743: [CodeGen][AArch64] Support arm_sve_vector_bits attribute

2020-08-12 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:3330 +// appendices to the Procedure Call Standard for the Arm Architecture, see: +// https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#appendix-c-mangling +void

[PATCH] D59615: [AArch64] When creating SISD intrinsic calls widen scalar args into a zero vectors, not undef

2020-08-11 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. I see two issues here: 1. We're still generating the wrong instruction. 2. The intrinsic is marked readnone, so any code that depends on whether it sets saturation flags is likely broken anyway. Given the layers of wrongness here, this seems like a marginal

[PATCH] D84992: [clang codegen] Use IR "align" attribute for static array arguments.

2020-08-10 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2503 + llvm::Align Alignment = + CGM.getNaturalTypeAlignment(ETy).getAsAlign(); + AI->addAttrs(llvm::AttrBuilder().addAlignmentAttr(Alignment));

[PATCH] D84992: [clang codegen] Use IR "align" attribute for static array arguments.

2020-08-10 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84992/new/ https://reviews.llvm.org/D84992 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D85599: [Clang] Consider __builtin_trap() and __builtin_debugtrap() as terminator

2020-08-10 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. Fixing a MachineVerifier issue by patching clang is generally wrong; if the IR is valid, the backend should do something sane with it. If the IR isn't valid, the IR Verifier should complain. Here, I'd say the IR is valid; adding a special case to the IR rules here

[PATCH] D82081: [z/OS] Add binary format goff and operating system zos to the triple

2020-08-07 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. clang changes look fine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82081/new/ https://reviews.llvm.org/D82081 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

2020-08-05 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. > If the intent is for getTypeInfo to always return sizes that are multiples of > the char size, then the design should be inverted and getTypeInfo should > simply be calling getTypeInfoInChars and multiply that result by the char > size. But that isn't how it works.

[PATCH] D85118: [clang][AArch64] Correct return type of Neon vqmovun intrinsics

2020-08-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. Probably the simplest way to verify return types would be using decltype. (`static_assert(std::is_same_v);`.) For arguments, maybe more complicated. I guess you could write a C++ class with conversion operators for every integer type, mark all of them except one "=

[PATCH] D85128: [Prototype][SVE] Support arm_sve_vector_bits attribute

2020-08-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. > I guess with what you're suggesting the bitcast could still be emitted there > but the cast operations could be limited in Sema to cases where ultimately > ConvertType would return a type that requires bitcasting, or are you saying > that could be avoided

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

2020-08-03 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1778 -std::pair FieldInfo = - Context.getTypeInfoInChars(D->getType()); -EffectiveFieldSize = FieldSize = FieldInfo.first; ebevhan wrote: > It seems that in factoring

[PATCH] D85128: [Prototype][SVE] Support arm_sve_vector_bits attribute

2020-08-03 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. Not going to write detailed review comments, but this looks like the right approach in general. One high-level thing to consider: we could still decide that in IR generation, we want to represent VLSTs registers using scalable vector types, like the original patch

[PATCH] D85118: [clang][AArch64] Correct return type of Neon vqmovun intrinsics

2020-08-03 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. Would it be worth looking into improving our general test coverage here? The fact that we don't have any test that verifies the actual signatures of the NEON intrinsics (which could be used to compare against other compilers) seems bad. Repository: rG LLVM Github

[PATCH] D84992: [clang codegen] Use IR "align" attribute for static array arguments.

2020-07-30 Thread Eli Friedman via Phabricator via cfe-commits
efriedma created this revision. efriedma added a reviewer: rjmccall. Herald added a project: clang. efriedma requested review of this revision. Without the "align" attribute, marking the argument dereferenceable is basically useless. See also D80166 . Fixes

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

2020-07-30 Thread Eli Friedman via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8dfb5d767e70: [clang codegen][AArch64] Use llvm.aarch64.neon.fcvtzs/u where its necessary (authored by efriedma). Changed prior to commit:

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

2020-07-27 Thread Eli Friedman via Phabricator via cfe-commits
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

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

2020-07-23 Thread Eli Friedman via Phabricator via cfe-commits
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

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

2020-07-20 Thread Eli Friedman via Phabricator via cfe-commits
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

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

2020-07-16 Thread Eli Friedman via Phabricator via cfe-commits
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

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

2020-07-14 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. In D83553#2151591 , @sdesmalen wrote: > In D83553#2148429 , @efriedma wrote: > > > > If you mean alloca's for single vectors > > > > I was really referring to the IR values themselves, not

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

2020-07-14 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7784 + // The __ARM_FEATURE_SVE_BITS macro must be defined when using this attribute. + auto = S.getPreprocessor(); + if (!PP.isMacroDefined("__ARM_FEATURE_SVE_BITS")) { c-rhodes wrote:

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

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

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

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

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

2020-07-09 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81583/new/ https://reviews.llvm.org/D81583 ___ cfe-commits mailing list

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

2020-07-09 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82085/new/ https://reviews.llvm.org/D82085

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

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

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

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

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

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

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

2020-07-08 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. > Should we remove the handling from llvm::ConstantFoldSelectInstruction It seems silly to remove the handling from InstSimplify, but not constant folding. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83360/new/

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

2020-07-08 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. Please also add testcases with select constant expressions, to test constant folding. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83360/new/ https://reviews.llvm.org/D83360

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

2020-07-08 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. In D81583#2139002 , @uweigand wrote: > In D81583#2137277 , @efriedma wrote: > > > I'm tempted to say this is a bugfix for the implementation of > > no_unique_address, and just fix it

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

2020-07-08 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:94 /// If it contains any dynamic allocas, returns false. static bool canTRE(Function ) { // Because of PR962, we don't TRE dynamic allocas. avl wrote: >

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

2020-07-07 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:94 /// If it contains any dynamic allocas, returns false. static bool canTRE(Function ) { // Because of PR962, we don't TRE dynamic allocas. If we're not

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

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

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

2020-06-30 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added reviewers: john.brawn, nikic. efriedma added subscribers: nikic, john.brawn. efriedma added a comment. I'm not sure what the status is of constrained fp on arm/aarch64; some patches went in, but I'm not sure what sort of testing was done. @john.brawn @nikic ? Otherwise, I think

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

2020-06-29 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. > This combines instructions, so I think it belongs into the InstCombine pass. > On the other hand, the f16 form of the intrinsics is not available on all > targets, so this combination cannot be applied unconditionally but it needs > to be gated depending on the

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

2020-06-29 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. In D81678#2120709 , @guiand wrote: > Could anyone point me to where I might need to make a change for that? LibCallSimplifier::optimizePow Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2020-06-25 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81282/new/ https://reviews.llvm.org/D81282

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

2020-06-25 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: compiler-rt/lib/builtins/CMakeLists.txt:142 powisf2.c powitf2.c subdf3.c Missed powitf? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81282/new/

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

2020-06-23 Thread Eli Friedman via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGbf8b63ed296c: [clang codegen] Fix alignment of Address for incomplete array pointer. (authored by efriedma). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2020-06-23 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. In D81678#2109059 , @nlopes wrote: > I'm a bit concerned with this patch as it increases the amount of UB that > LLVM exploits without any study of the impact. > For example, right now it's ok do this with clang (not with

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

2020-06-23 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:825 + // The local stack holds all alloca instructions and all byval arguments. + AllocaDerivedValueTracker Tracker; + for (Argument : F.args()) { avl wrote:

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

2020-06-23 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:825 + // The local stack holds all alloca instructions and all byval arguments. + AllocaDerivedValueTracker Tracker; + for (Argument : F.args()) { avl wrote:

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

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

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

2020-06-23 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:825 + // The local stack holds all alloca instructions and all byval arguments. + AllocaDerivedValueTracker Tracker; + for (Argument : F.args()) { avl wrote:

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

2020-06-23 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:825 + // The local stack holds all alloca instructions and all byval arguments. + AllocaDerivedValueTracker Tracker; + for (Argument : F.args()) { avl wrote:

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

2020-06-22 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:825 + // The local stack holds all alloca instructions and all byval arguments. + AllocaDerivedValueTracker Tracker; + for (Argument : F.args()) { avl wrote:

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

2020-06-22 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:825 + // The local stack holds all alloca instructions and all byval arguments. + AllocaDerivedValueTracker Tracker; + for (Argument : F.args()) { avl wrote:

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

2020-06-22 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp:130 +IsNocapture = true; + else if (Function *CalledFunction = CB.getCalledFunction()) { +if (CalledFunction->getBasicBlockList().size() > 0 &&

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

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

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

2020-06-22 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79167/new/ https://reviews.llvm.org/D79167 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

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

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

2020-06-19 Thread Eli Friedman via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGc310bf8256f8: [Sema] Comparison of pointers to complete and incomplete types (authored by pestctrl, committed by efriedma). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

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

  1   2   3   4   5   6   7   8   9   10   >