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

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

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

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79039/new/ https://reviews.llvm.org/D79039 ___ cfe-commits mailing list cfe-commi

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

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

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

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

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

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

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

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM with one minor comment Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7739 + +// Index needs to be passed as scaled offset. +llvm::Type *MemEltTy = SVEBuiltinMe

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79087/new/ https://reviews.llvm.org/D79087 ___ cfe-commits mailing list cfe-commi

[PATCH] D76689: [Sema][SVE] Fix handling of initialisers for built-in SVE types

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

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

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 it's necessary (authored by efriedma). Changed prior to commit: https:

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

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 http

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

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] D85128: [Prototype][SVE] Support arm_sve_vector_bits attribute

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 did

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

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 o

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

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 completely?

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

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] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

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] D82081: [z/OS] Add binary format goff and operating system zos to the triple

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 https://lists

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

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 wou

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

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 https://lists.llvm.org/cgi-

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

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)); rj

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

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 improvement

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

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 CXXNameMangler::mang

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

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 th

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

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 2>&

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

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 CH

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

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 +IsInc

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

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78750/new/ https://reviews.llvm.org/D78750 ___ cfe-commits mailing list cfe-commi

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

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

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

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78756/new/ https://reviews.llvm.org/D78756 ___ cfe-commits mailing list cfe-commi

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

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

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

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

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

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

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

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

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

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

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

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

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

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM with one minor comment Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7813 + llvm::ScalableVectorType *Ty = getSVEType(TypeFlags); + return Builder.CreateBitCast(Call

[PATCH] D79478: [CodeGen][SVE] Lowering of shift operations with scalable types

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

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

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78812/new/ https://reviews.llvm.org/D78812 ___ cfe-commits mailing list cfe-commi

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

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

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

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

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

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

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

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

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

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79579/new/ https://reviews.llvm.org/D79579 ___ cfe-commits mailing list cfe-commi

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79357/new/ https://reviews.llvm.org/D79357 ___ cfe-commits mailing list cfe-commi

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

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

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

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

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

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

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

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

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

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

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

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

[PATCH] D40256: [ARM] disable FPU features when using soft floating point.

efriedma added inline comments. Comment at: test/Driver/arm-mfpu.c:301 +// RUN: | FileCheck --check-prefix=CHECK-SOFT-ABI-FP %s +// R-UN: %clang -target armv4-linux-gnueabi -mfloat-abi=soft -mfpu=none %s -### -c 2>&1 \ +// R-UN: | FileCheck --check-prefix=CHECK-SOFT-ABI-FP %

[PATCH] D40594: [InstCombine] miscompile of __builtin_fmod

efriedma added a comment. By many years of precedent, the "frem" instruction is supposed to match the C fmod(), as opposed to something else like the C99 remainder(); probably worth clarifying in LangRef. https://reviews.llvm.org/D40594 ___ cfe-co

[PATCH] D39627: Fix vtable not receiving hidden visibility when using push(visibility)

efriedma added a comment. You probably just need to pass a "-triple" flag so we don't use Windows mangling or something like that. Repository: rL LLVM https://reviews.llvm.org/D39627 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D40256: [ARM] disable FPU features when using soft floating point.

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D40256 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26

efriedma added a comment. > _Float128 is only *sometimes* the same type as __float128. But we don't have hppa or IA-64 backends, so we're fine for now, I think. :) Comment at: lib/Frontend/InitPreprocessor.cpp:817 DefineFloatMacros(Builder, "LDBL", &TI.getLongDoubleFormat()

[PATCH] D33765: Show correct column nr. when multi-byte utf8 chars are used.

efriedma added a comment. Still worried about the effect on tools which parse clang diagnostics... please send a message to cfe-dev. Hopefully we'll get responses there. https://reviews.llvm.org/D33765 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D40698: [ubsan] Diagnose noreturn functions which return

efriedma added inline comments. Comment at: lib/CodeGen/CGCall.cpp:2756 SourceLocation EndLoc) { + if (FI.isNoReturn()) { +// Noreturn functions don't return. Unfortunately, this won't catch cases where the caller ha

[PATCH] D40698: [ubsan] Diagnose noreturn functions which return

efriedma added inline comments. Comment at: lib/CodeGen/CGCall.cpp:2756 SourceLocation EndLoc) { + if (FI.isNoReturn()) { +// Noreturn functions don't return. vsk wrote: > efriedma wrote: > > Unfortunately, this won'

[PATCH] D40044: [CodeGen] convert math libcalls/builtins to equivalent LLVM intrinsics

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D40044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[PATCH] D40566: Check if MemberExpr arguments are type-dependent.

efriedma added a comment. The part which seems to be throwing you off is "or the class member access expression refers to a member of an unknown specialization". A "member of an unknown specialization" is defined in [temp.dep.type] in the standard. https://reviews.llvm.org/D40566 _

[PATCH] D28820: Warn when calling a non interrupt function from an interrupt on ARM

efriedma added a comment. > What is the best way to modify the code for this compiler change ? Currently, the "interrupt" attribute only has an effect on functions, not function pointers, so your code won't work the way you want. It's a bug that we don't emit a warning for this. Currently, th

[PATCH] D40940: [ubsan] Use pass_object_size info in bounds checks

efriedma added a comment. > It's interesting to me that these array-bound checks don't seem to use > @llvm.objectsize in some form already. That would be a cool experiment. That said, one of the upsides of the current ubsan is that whether it will produce a diagnostic is predictable (as long a

[PATCH] D40940: [ubsan] Use pass_object_size info in bounds checks

efriedma added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:833 + // Arrays don't have pass_object_size attributes, but if they have a constant + // size modifier it's the array size (C99 6.5.7.2p1). + if (auto *DecayedArrayTy = dyn_cast(ParamDecl->getType())) -

[PATCH] D41044: Implementation of -fextend-lifetimes and -fextend-this-ptr to aid with debugging of optimized code

efriedma added a comment. This might be something we want to turn on automatically for -Og. https://reviews.llvm.org/D41044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41140: [Coverage] Always emit unused coverage mappings in the same order.

efriedma created this revision. efriedma added a reviewer: vsk. efriedma added a project: clang. Non-determinism is confusing at best. Repository: rC Clang https://reviews.llvm.org/D41140 Files: lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h Index: lib/CodeGen/CodeGenModule.

[PATCH] D41140: [Coverage] Always emit unused coverage mappings in the same order.

This revision was automatically updated to reflect the committed changes. Closed by commit rL320533: [Coverage] Always emit unused coverage mappings in the same order. (authored by efriedma, committed by ). Changed prior to commit: https://reviews.llvm.org/D41140?vs=126640&id=126644#toc Reposi

[PATCH] D39963: [RISCV] Add initial RISC-V target and driver support

efriedma added reviewers: mgorny, efriedma, bkramer, sylvestre.ledru. efriedma added a comment. Adding some reviewers to hopefully find someone comfortable reviewing the Linux multilib bits. (Not sure I'm adding the right people; this stuff hasn't been touched for a while, as far as I can tell.

[PATCH] D41149: [CodeGen] Specialize mixed-sign mul-with-overflow (fix PR34920)

efriedma added inline comments. Comment at: test/CodeGen/builtins-overflow.c:402 + int result; + if (__builtin_mul_overflow(y, x, &result)) +return LongLongErrorCode; I think the rules for __builtin_mul_overflow say you have to check whether the truncate c

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

efriedma added a comment. There's no way the calling convention can change based on whether you're calling a function vs. a function pointer. I can't explain why MSVC is generating different code. I think we should just ignore it, at least for now. Repository: rG LLVM Github Monorepo CHAN

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

efriedma added a comment. If we don't specifically call out the deprecation period in the diagnostic, usage will grow beyond what we expect. (Most people don't read the release notes; they'll just see it's possible to turn off the error message, and turn it off.) If we're okay with people dec

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

efriedma added a comment. I'm not really a fan of the argument of "this was accepted as a DR, therefore new versions of clang have to reject code that old versions accepted". Regardless of what the C++ committee does, our commitment to our users is to ensure that code written against "-std=c++

[PATCH] D132843: [RISCV] Ensure target features get passed to the LTO linker for RISC-V

efriedma added a comment. I'd prefer to use an approach with less moving parts: the bitcode should contain enough information to generate code without specifying anything on the command-line that wouldn't normally be passed to the linker. Among other issues, it's hard to understand the intende

[PATCH] D132848: [clang] Fix checking for emulated TLS in shouldAssumeDSOLocal in CodeGen

efriedma added a comment. I'm not really happy with the way this implicitly ties the implementation of this function to the implementation of TargetMachine::useEmulatedTLS(). I'd prefer to just make clang always explicitly compute whether EmullatedTLS is enabled, then always set ExplicitEmulat

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

efriedma added a comment. > We have the VariableArrayType to represent a VLA type and I think we're using > that type when we should be using a variably modified type The clang AST for function signatures encodes two types: the type as written, and the type after promotion. Semantic analysis a

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

efriedma added a comment. The reason struct returns require register shuffling is that AArch64 passes the sret pointer in x8 (i.e. RAX), but the x64 calling convention expects in in RCX (i.e. x0). Have you tried to see if the Microsoft-generated thunk actually works? I found at least one bug

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

efriedma added a comment. In D132952#3759226 , @aaron.ballman wrote: >> We could theoretically mess with the AST representation somehow, but I don't >> see any compelling reason to. We probably just want to emit the >> warn_vla_used diagnostic somewhe

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

efriedma added a comment. As a practical matter, there isn't any reason to force variably modified parameters to make a function variably modified. The types of parameters aren't visible in the caller of a function; we only check the compatibility for calls. Trying to treat `void (*)(int, int

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

efriedma added inline comments. Comment at: clang/docs/ReleaseNotes.rst:141 +- Implemented `WG14 N2562 `_. + Clang will now consider default argument promotions in printf, and remove unnecessary warnings. --

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

efriedma updated this revision to Diff 457457. efriedma added a comment. Rebased Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125418/new/ https://reviews.llvm.org/D125418 Files: clang/include/clang/AST/Mangle.h clang/lib/AST/MicrosoftMangle.c

[PATCH] D125419: [Arm64EC 7/?] clang side of Arm64EC varargs ABI.

efriedma updated this revision to Diff 457459. efriedma added a reviewer: rjmccall. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125419/new/ https://reviews.llvm.org/D125419 Files: clang/lib/CodeGen/TargetInfo.cpp clang/test/CodeGen/arm64ec.c

[PATCH] D125419: [Arm64EC 7/?] clang side of Arm64EC varargs ABI.

efriedma added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:2457 +/*IsVectorCall=*/false, /*IsRegCall=*/false); + } + rjmccall wrote: > Hmm. Doesn't EC ABI lowering need to preserve this same state, or else > you'll get inc

[PATCH] D132843: [RISCV] Ensure target features get passed to the LTO linker for RISC-V

efriedma added a comment. > So we need to keep ABI in somewhere and read that at LTO phase, the most > ideal place is the module flags. We already did that[6], but that comes with > a problem is it's too late to update datalayout when we start to read a > module, because LLVM will use datalayou

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

efriedma added reviewers: rjmccall, mstorsjo, dpaoliello, rnk. efriedma added a comment. I think I'd like to continue moving forward with approximately this approach, at least for the moment. As far as I know, D132926 solves the remaining issues with translati

[PATCH] D125419: [Arm64EC 7/?] clang side of Arm64EC varargs ABI.

efriedma planned changes to this revision. efriedma added a comment. Just realized I forgot to add tests for va_arg. Comment at: clang/lib/CodeGen/TargetInfo.cpp:2457 +/*IsVectorCall=*/false, /*IsRegCall=*/false); + } + rjmccall wrote: > ef

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