[PATCH] D151923: [APFloat] Add APFloat semantic support for TF32

majnemer added inline comments. Comment at: llvm/include/llvm/ADT/APFloat.h:190 +// greater throughput than single precision (32-bit) formats. +S_FloatTF32, Hmm, this says improved precision than half but the semantics you gave say 11 digits? Does NVI

[PATCH] D146441: [APFloat] Add E4M3B11FNUZ

majnemer added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:848 case APFloat::S_Float8E4M3FNUZ: + case APFloat::S_Float8E4M3B11FNUZ: llvm_unreachable("Tried to mangle unexpected APFloat semantics"); aaron.ballman wrote: > Why are there

[PATCH] D146441: [APFloat] Add E4M3B11FNUZ

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2f086f265bf9: [APFloat] Add E4M3B11FNUZ (authored by majnemer). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D146441: [APFloat] Add E4M3B11FNUZ

majnemer updated this revision to Diff 506694. majnemer added a comment. Fix a small typo in PyFloat8E4M3B11FNUZType Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146441/new/ https://reviews.llvm.org/D146441 Files: clang/lib/AST/MicrosoftMangle.

[PATCH] D146441: [APFloat] Add E4M3B11FNUZ

majnemer created this revision. majnemer added a reviewer: reedwm. Herald added subscribers: Moerafaat, zero9178, bzcheeseman, sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, antiagains

[PATCH] D141347: Remove the ThreadLocal template from LLVM.

majnemer accepted this revision. majnemer added a comment. This revision is now accepted and ready to land. LGTM Comment at: llvm/lib/Support/CrashRecoveryContext.cpp:23 struct CrashRecoveryContextImpl; - -sys::ThreadLocal &getCurrentContext() { - static sys::ThreadLocal Curr

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

majnemer added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:12452 +int Ilogb; +if (APFloat::opStatus St = ilogb(F, Ilogb); !isConstantOpStatus(St)) + return false; Izaron wrote: > jcranmer-intel wrote: > > Izaron wrote: > > > aaron.bal

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

majnemer added a comment. Your example is different from mine as it nests the constexpr variable inside the function rather than having it at translation-unit scope. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117569/new/ https://reviews.llvm.org/D117569

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

majnemer added a comment. In D117569#3258307 , @zahiraam wrote: > In D117569#3257121 , @majnemer > wrote: > >> I have a question regarding how this work with respect to the dllimport >> semantics known by the li

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

majnemer added inline comments. Comment at: clang/test/CodeGenCXX/dllimport.cpp:2 // RUN: %clang_cc1 -disable-noundef-analysis -triple i686-windows-msvc -fno-rtti -fno-threadsafe-statics -fms-extensions -emit-llvm -std=c++1y -O0 -o - %s -DMSABI -w | FileCheck --check-prefix=

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

majnemer added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:2219 + Info.getLangOpts().CPlusPlus && !isForManglingOnly(Kind) && + Var->hasAttr()) // FIXME: Diagnostic! The summary and the bug both mention dllimport but

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

majnemer requested changes to this revision. majnemer added a comment. This revision now requires changes to proceed. I have a question regarding how this work with respect to the dllimport semantics known by the linker. IIUC, we will now allow a program like: extern int __declspec(dllimport)

[PATCH] D115456: [MS] Implement on-demand TLS initialization for Microsoft CXX ABI

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG072e2a7c67b7: [MS] Implement on-demand TLS initialization for Microsoft CXX ABI (authored by momo5502, committed by majnemer). Repository: rG LLVM

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

majnemer accepted this revision. majnemer added a comment. This revision is now accepted and ready to land. Looks great! Please give others some time to review it as it is a holiday season... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115456/new/ https://reviews.llvm.org/D115456 __

[PATCH] D116020: [clang][#52782] Bail on incomplete parameter type in stdcall name mangling

majnemer added a comment. I wonder if we should have different behavior for MSVC targets. If I do: class Incomplete; extern "C" int __stdcall Fn(int, Incomplete, long long); auto fnptr = &Fn; MSVC generates: EXTRN _Fn@12:PROC It appears that they skip over incomplete types. Should

[PATCH] D114425: [clang] Add __builtin_bswap128

majnemer added a comment. In D114425#3217478 , @Quuxplusone wrote: > In D114425#3216802 , @majnemer > wrote: > >> OOC, how hard would it be to generalize this builtin a little? It is nice >> that we have builti

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

majnemer added a comment. In D115456#3217595 , @momo5502 wrote: > In D115456#3216811 , @majnemer > wrote: > >> This is looking great! Just a few more questions. >> >> What is the behavior with something like: >>

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

majnemer added a comment. This is looking great! Just a few more questions. What is the behavior with something like: thread_local int x = 2; int f() { return x; } I'm wondering if we need to move this logic

[PATCH] D114425: [clang] Add __builtin_bswap128

majnemer added a comment. OOC, how hard would it be to generalize this builtin a little? It is nice that we have builtins like `__builtin_add_overflow` which do the right thing regardless of their input. It seems like it would be nice if we started to expose more intrinsics which did the right

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

majnemer added inline comments. Comment at: clang/lib/CodeGen/MicrosoftCXXABI.cpp:404 bool usesThreadWrapperFunction(const VarDecl *VD) const override { -return false; +return true; } Should this depend on the MSVC compatibility version? CHANGES S

[PATCH] D115456: Implement on-demand TLS initialization for Microsoft CXX ABI

majnemer added a comment. Is this a new feature in MSVC? Seems like it might be. If so, should it be predicated on `isCompatibleWithMSVC(1925)` or something? CHANGES SINCE LAS

[PATCH] D88905: [Clang] Allow "ext_vector_type" applied to Booleans

majnemer added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2782 + +// construct the vector of 'unsigned char' type +QualType CharVecTy = Ctx.getVectorType(Ctx.CharTy, NumVectorBytes, The code as written seems to be 'char' type, not 'uns

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

majnemer added inline comments. Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:4121-4125 - if (isa(TrueVal)) // select ?, undef, X -> X -return FalseVal; - if (isa(FalseVal)) // select ?, X, undef -> X -return TrueVal; - Can we still do these

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

majnemer added inline comments. Comment at: clang/include/clang/AST/Stmt.h:1783 + + bool IsSideEntry() const { return SideEntry; } + void setSideEntry() { SideEntry = true; } I think this should be isSideEntry to be consistent. Comment at: cl

[PATCH] D76077: [ARM] Add __bf16 as new Bfloat16 C Type

majnemer added inline comments. Comment at: clang/include/clang-c/Index.h:3254 CXType_FirstBuiltin = CXType_Void, CXType_LastBuiltin = CXType_ULongAccum, Should this be: CXType_LastBuiltin = CXType_BFloat16, Comment at: clang/lib/AST

[PATCH] D77777: [nvptx] Add `nvvm.texsurf.handle` internalizer.

majnemer added a comment. In D7#1975618 , @hliao wrote: > In D7#1975406 , @tra wrote: > > > In D7#1975178 , @hliao wrote: > > > > > the 1st argument in `llvm.nvvm

[PATCH] D66827: Add support for MS qualifiers __ptr32, __ptr64, __sptr, __uptr.

majnemer added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:1718 QualType PointeeType) { - if (PointersAre64Bit && - (PointeeType.isNull() || !PointeeType->isFunctionType())) + // Check if this is

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

majnemer added inline comments. Comment at: test/libcxx/numerics/truncating_cast.pass.cpp:12 +// Test the conversion function that truncates floating point types to the +// closes representable value for the specified integer type, or +// numeric_limits::max()/min() if the value

[PATCH] D56466: [CodeGen] Clarify comment about COFF common symbol alignment

majnemer added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:3765-3766 + + // MSVC doesn't support alignments greater than 32 for common symbols, so + // symbols with greater alignment requirements cannot be common. Non-MSVC COFF + // environments support arbitra

[PATCH] D56391: Limit COFF 'common' emission to <=32 alignment types.

majnemer added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:3766 + // in common. + if (Context.getTargetInfo().getTriple().isOSBinFormatCOFF() && + Context.getTypeAlignIfKnown(D->getType()) > 32) I think this should be isKnownWindowsMSVCEnvir

[PATCH] D55853: Ignore ConstantExpr in IgnoreParenNoopCasts

majnemer added inline comments. Comment at: clang/lib/AST/Expr.cpp:2722-2725 +if (ConstantExpr *CE = dyn_cast(E)) { + E = CE->getSubExpr(); + continue; +} Just curious, why not go even further and use FullExpr rather than ConstantExpr? CHANGES

[PATCH] D55715: Add AddressSpace mangling to MS mode

majnemer added inline comments. Comment at: lib/AST/MicrosoftMangle.cpp:1806-1836 +Extra.mangleSourceName("AS_"); +Extra.mangleIntegerLiteral(llvm::APSInt::getUnsigned(TargetAS), + /*IsBoolean*/ false); + } else { +switch (AS) { +def

[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C

majnemer added inline comments. Comment at: lib/AST/DeclBase.cpp:1711-1712 + // contexts are always skipped. + while (SkipRecords ? Ctx->isTransparentContext() || Ctx->isRecord() + : Ctx->isTransparentContext()) Ctx = Ctx->getParent();

[PATCH] D52499: [clang-cl] Make /Gs imply default stack probes, not /Gs0 (PR39074)

majnemer accepted this revision. majnemer added a comment. This revision is now accepted and ready to land. FWIW, Microsoft's newest documentation does not say that `/O2` and `/O1` imply `/Gs`: https://docs.microsoft.com/en-us/cpp/build/reference/o1-o2-minimize-size-maximize-speed?view=vs-2017

[PATCH] D50877: [MS] Mangle a hash of the main file path into anonymous namespaces

majnemer added a comment. How does MSVC handle this case? What mangled name does it generate? https://reviews.llvm.org/D50877 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49354: [MinGW] Automatically mangle Windows-specific entry points as C

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

[PATCH] D47875: [MS ABI] Mangle unnamed empty enums (PR37723)

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

[PATCH] D47875: [MS ABI] Mangle unnamed empty enums (PR37723)

majnemer added inline comments. Comment at: lib/AST/MicrosoftMangle.cpp:888-891 auto EnumeratorI = ED->enumerator_begin(); -assert(EnumeratorI != ED->enumerator_end()); -Name += "getName(); +if (EnumeratorI == ED->enumerator_end()) { + Na

[PATCH] D44931: [WebAssembly] Use Windows EH instructions for Wasm EH

majnemer added inline comments. Comment at: lib/CodeGen/CGException.cpp:1173 +cast(CatchStartBlock->getFirstNonPHI()); +CurrentFuncletPad = CPI; + } aheejin wrote: > majnemer wrote: > > Hmm, why is this done? Won't RestoreCurrentFuncletPad undo this?

[PATCH] D44931: [WebAssembly] Use Windows EH instructions for Wasm EH

majnemer added inline comments. Comment at: lib/CodeGen/CGException.cpp:1164 + CurrentFuncletPad); + llvm::BasicBlock *CatchStartBlock = nullptr; + if (EHPersonality::get(*this).isWasmPersonality()) { Maybe this should be called WasmCatchStartBlock?

[PATCH] D45839: [analyzer] Add support for WebKit "unified sources".

majnemer added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h:145-147 + if (Name.endswith_lower(".c") || Name.endswith_lower(".cpp") || + Name.endswith_lower(".cc") || Name.endswith_lower(".cxx") || + Name.en

[PATCH] D45738: Add Microsoft mangling for _Float16, similar to technique used for _Complex

majnemer accepted this revision. majnemer added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D45738 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[PATCH] D45174: non-zero-length bit-fields should make a class non-empty

majnemer added a comment. In https://reviews.llvm.org/D45174#1055125, @rsmith wrote: > In https://reviews.llvm.org/D45174#1055048, @rsmith wrote: > > > I wonder if we can delete the `getNonVirtualSize()` check now -- I don't > > see any way that an empty class can have a nonzero nvsize except by

[PATCH] D45112: [MS] Emit vftable thunks for functions with incomplete prototypes

majnemer added a comment. Does this help PR25641? https://reviews.llvm.org/D45112 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44931: [WebAssembly] Use Windows EH instructions for Wasm EH

majnemer added a comment. Some quick first pass comments. Comment at: lib/CodeGen/CGCleanup.cpp:985 +// does not have a runtime support for that. +if (!Personality.usesFuncletPads() || Personality.isWasmPersonality()) { + EHStack.pushTerminate(); I

[PATCH] D44327: ObjCARC: teach the cloner about funclets

majnemer accepted this revision. majnemer added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/Transforms/ObjCARC/ObjCARCOpts.cpp:692 + DenseMap &BlockColors) { + auto *CI = dyn_cast(&I); + assert(CI && "CloneCallInst must r

[PATCH] D44327: ObjCARC: teach the cloner about funclets

majnemer added inline comments. Comment at: lib/Transforms/ObjCARC/ObjCARCOpts.cpp:701 + + if (auto *CleanupPad = dyn_cast(BB.getFirstNonPHI())) +OpBundles.emplace_back("funclet", CleanupPad); What if the cleanuppad was introduced in a block which branched t

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

majnemer added a comment. In https://reviews.llvm.org/D43576#1016512, @rsmith wrote: > Do we need to also track whether the argument is a pointer or reference to a > UUID (and also the cv-qualifiers)? For the `Declaration` case, we track this > by tracking the corresponding parameter type; the

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

majnemer added a comment. In https://reviews.llvm.org/D43576#1016418, @zahiraam wrote: > In https://reviews.llvm.org/D43576#1016295, @majnemer wrote: > > > We should really, really avoid making this sort of change without first > > trying to desugar uuidof into a reference to a variable. That wo

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

majnemer added a comment. We should really, really avoid making this sort of change without first trying to desugar uuidof into a reference to a variable. That would solve a ton of problems, problems like this one. https://reviews.llvm.org/D43576

[PATCH] D43184: [WIP] [ItaniunCXXABI] Add an option for loading the type info vtable with dllimport

majnemer added a comment. Do I understand correctly that this workarounds a feature missing in lld? Does MinGW emit the same sorts of object files as clang in these scenarios? Repository: rC Clang https://reviews.llvm.org/D43184 ___ cfe-commits

[PATCH] D43033: [WinEH] Put funclet bundles on inline asm calls

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

[PATCH] D42758: Support `#pragma comment(lib, "name")` in the frontend for ELF

majnemer added a comment. docs/LanguageExtensions.html should be updated to mention that we support this extension on all ELF targets. Repository: rC Clang https://reviews.llvm.org/D42758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D42641: [MinGW] Emit typeinfo locally for dllimported classes without key functions

majnemer added inline comments. Comment at: lib/CodeGen/CGVTables.cpp:887-888 + if (CGM.getTriple().isWindowsGNUEnvironment() && RD->hasAttr()) +return true; + Maybe a comment like "VTables of classes declared as dllimport are always considered to be ext

[PATCH] D42508: AST: support protocol conformances on id/class/interfaces in MS ABI

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

[PATCH] D42248: Always allow "#pragma region".

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

[PATCH] D42508: AST: support protocol conformances on id/class/interfaces in MS ABI

majnemer added inline comments. Comment at: lib/AST/MicrosoftMangle.cpp:2459-2465 + if (T->isObjCId()) +mangleSourceName("objc_object"); + else if (T->isObjCClass()) +mangleSourceName("objc_class"); + else +mangleSourceName(T->getInterface()->getName()); +

[PATCH] D42343: [coroutines] Fix application of NRVO to Coroutine "Gro" or return object.

majnemer added inline comments. Comment at: test/CodeGenCoroutines/coro-gro-nrvo.cpp:17 +using SizeT = decltype(sizeof(int)); +void* operator new(SizeT __sz, const std::nothrow_t&) noexcept; +void operator delete(void* __p, const std::nothrow_t&) noexcept; `Size

[PATCH] D42248: [LangOpts] Add a LangOpt to represent "#pragma region" support.

majnemer added a comment. Why not always support the pragma regardless of the compiler mode? Our "support" for it just ignores it anyway... https://reviews.llvm.org/D42248 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.or

[PATCH] D41032: [CodeGen][X86] Implement _InterlockedCompareExchange128 intrinsic

majnemer added inline comments. Comment at: llvm/tools/clang/lib/CodeGen/CGBuiltin.cpp:8439 + +llvm::Type *Int128Ty = llvm::IntegerType::get(getLLVMContext(), 128); +llvm::Type *Int128PtrTy = Int128Ty->getPointerTo(); Builder.getInt128Ty() https://revie

[PATCH] D40071: [libcxx] Implement exception_ptr on Windows without msvcprt.dll

majnemer added inline comments. Comment at: src/support/runtime/exception_pointer_msvc.ipp:119-123 +#ifdef _M_AMD64 +RtlPcToFileHeader( +reinterpret_cast(const_cast(throw_info)), +&image_base); +#endif Can't you use the image_base field in thr

[PATCH] D40023: [RISCV] Implement ABI lowering

majnemer added a comment. In https://reviews.llvm.org/D40023#933466, @asb wrote: > In https://reviews.llvm.org/D40023#933464, @majnemer wrote: > > > So how does something like the following work: > > > > union U { float f; int i; }; > > void f(union U u); > > > > > > The flattening describ

[PATCH] D40023: [RISCV] Implement ABI lowering

majnemer added a comment. So how does something like the following work: union U { float f; int i; }; void f(union U u); The flattening described in https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md#hardware-floating-point-calling-convention makes sense for arrays and s

[PATCH] D40218: [Clang] Add __builtin_launder

majnemer added a comment. A test with restrict and __restrict might be interesting. https://reviews.llvm.org/D40218 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

majnemer added a comment. It'd be good to have tests which have alignment directives on bitfields. https://reviews.llvm.org/D39347 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38704: [libunwind] Abstract rwlocks into a class, provide a SRW lock implementation for windows

majnemer accepted this revision. majnemer added a comment. LGTM https://reviews.llvm.org/D38704 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38704: [libunwind] Emulate pthread rwlocks via SRW locks for windows

majnemer added a comment. I don't think we should depend on LLVM for the locking stuff. This sort of infrastructure is in the same bucket as the demangler which we haven't really solved. I *do* find it weird to do it this way though. I think you should have some mutex functions which include r

[PATCH] D38656: [CGExprScalar] In EmitCompare trunc the result if it has different type as E->getType()

majnemer added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:3223-3224 + // crash later. + llvm::IntegerType *ResultTy = + dyn_cast(Result->getType()); + if ((ResultTy->getBitWidth() > 1) && Is this clang-format'

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list= to match gcc options.

majnemer added inline comments. Comment at: lib/CodeGen/CodeGenFunction.cpp:455 +for (const auto &FileMatch : PathSearch) { + if(FunctionDeclPath.find(FileMatch) != std::string::npos) { +return false; Space after if. Comment at

[PATCH] D37624: add support for -fno-instrument-functions and -finstrument-functions-exclude-{file, function}-list= to match gcc options.

majnemer added inline comments. Comment at: lib/CodeGen/CodeGenFunction.cpp:463 + + // Skip demangling if decl is extern "C" + if (ActualFuncDecl && !ActualFuncDecl->isExternC()) { Is this comment still correct? Comment at: lib/CodeGen/CodeGe

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

majnemer added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.h:32 #include "clang/Basic/XRayLists.h" +#include "clang/Frontend/CodeGenOptions.h" #include "llvm/ADT/DenseMap.h" Any reason to do this? I'd just keep getNestedExpressionLocationsEnabl

[PATCH] D33852: Enable __declspec(selectany) on linux

majnemer added inline comments. Comment at: include/clang/Basic/AttrDocs.td:3154 + +def SelectAnyDocs : Documentation { + let Content = [{This attribute makes global symbol have a weak definition aaron.ballman wrote: > Prazek wrote: > > aaron.ballman wrote: > >

[PATCH] D37206: [ItaniumCXXABI] Always use linkonce_odr linkage for RTTI data on MinGW

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

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

majnemer added a comment. In https://reviews.llvm.org/D37042#849726, @majnemer wrote: > I'd expect this to be more limited. > > Something like, "if we have a BinaryOperator of + between a CStyleCastExpr of > NullToPointer and 'X', emit inttoptr(X)" Although maybe that is too strict... I guess

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

majnemer added a comment. I'd expect this to be more limited. Something like, "if we have a BinaryOperator of + between a CStyleCastExpr of NullToPointer and 'X', emit inttoptr(X)" https://reviews.llvm.org/D37042 ___ cfe-commits mailing list cfe-c

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

majnemer added inline comments. Comment at: include/clang/AST/Expr.h:3816 +/// __builtin_LINE(), __builtin_COLUMN(), __builtin_FUNCTION(), or +/// __BUILTIN_FILE() +class SourceLocExpr final : public Expr { __BUILTIN_FILE -> __builtin_FILE Comm

[PATCH] D35782: [C++2a][Preprocessor] Implement p0306 __VA_OPT__ (Comma omission and comma deletion)

majnemer added inline comments. Comment at: include/clang/Lex/TokenLexer.h:169 + bool PasteTokens(Token &Tok, + llvm::ArrayRef AltTokens = llvm::ArrayRef(), + unsigned int *const AltCurTokenIdx = nullptr); I think `llvm::Array

[PATCH] D35538: [CodeGen][ARM] ARM runtime helper functions are not always soft-fp

majnemer added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:5625 + // The Run-time ABI for the ARM Architecture section 4.1.2 requires + // AEABI-complying FP helper functions to use the base AAPCS + // These AEABI functions are expanded in the ARM llvm backend, all

[PATCH] D35824: [Sema] Implicitly apply the may_alias attribute to std::byte

This revision was automatically updated to reflect the committed changes. Closed by commit rL309058: [CodeGen] Correctly model std::byte's aliasing properties (authored by majnemer). Changed prior to commit: https://reviews.llvm.org/D35824?vs=108166&id=108185#toc Repository: rL LLVM https:/

[PATCH] D35824: [Sema] Implicitly apply the may_alias attribute to std::byte

majnemer updated this revision to Diff 108166. majnemer added a comment. - Address review comments https://reviews.llvm.org/D35824 Files: include/clang/AST/Type.h lib/AST/Type.cpp lib/CodeGen/CodeGenTBAA.cpp test/CodeGenCXX/std-byte.cpp Index: test/CodeGenCXX/std-byte.cpp

[PATCH] D35824: [Sema] Implicitly apply the may_alias attribute to std::byte

majnemer updated this revision to Diff 108105. majnemer added a comment. - Address review feedback. https://reviews.llvm.org/D35824 Files: include/clang/AST/Type.h lib/AST/Type.cpp lib/CodeGen/CodeGenTBAA.cpp test/CodeGenCXX/std-byte.cpp Index: test/CodeGenCXX/std-byte.cpp ===

[PATCH] D35824: [Sema] Implicitly apply the may_alias attribute to std::byte

majnemer created this revision. std::byte, when defined as an enum, needs to be given special treatment with regards to its aliasing properties. An array of std::byte is allowed to be used as storage for other types. This fixes PR33916. https://reviews.llvm.org/D35824 Files: lib/Sema/SemaDec

[PATCH] D34972: [CodeGen] Propagate dllexport to thunks

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

[PATCH] D34972: [CodeGen] Propagate dllexport to thunks

majnemer added a comment. OK, so we are exporting the thunks so that the linker will generate import thunks for the thunks. I think that we should have a comment to that effect near the code you added. https://reviews.llvm.org/D34972 ___ cfe-commi

[PATCH] D35715: Preserve typedef names in debug info for template type parameters

majnemer added a comment. This might be a silly question but why not do this by default? https://reviews.llvm.org/D35715 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35472: Implement P0463R1: "Endian just Endian"

majnemer added inline comments. Comment at: test/std/utilities/meta/meta.type.synop/endian.pass.cpp:39-42 +union { +uint32_t i; +char c[4]; +} u = {0x01020304}; This is undefined behavior as-is because you are reading from a union member

[PATCH] D35180: Expose the Clang::QualType to llvm::Type conversion functions

majnemer added inline comments. Comment at: lib/CodeGen/CodeGenABITypes.cpp:71 + assert(FD != nullptr && "Expected a non-null function declaration!"); + llvm::Type* T = CGM.getTypes().ConvertFunctionType(FD->getType(), FD); + Pointers lean right.

[PATCH] D34714: [MS] Don't statically initialize dllimport member function pointers

majnemer added a comment. In https://reviews.llvm.org/D34714#793205, @rnk wrote: > Did you locally add a test case for the dllimport member function pointer > template argument? Arg, yes. Forgot to add the file... Comment at: lib/Sema/SemaTemplate.cpp:5704 + else +NPV

[PATCH] D34714: [MS] Don't statically initialize dllimport member function pointers

majnemer created this revision. r306137 made dllimport pointers to member functions non-constant. This is correct because a load must be executed to resolve any dllimported data. However, r306137 did not account for the use of dllimport member function pointers used as template arguments. This ch

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

majnemer accepted this revision. majnemer added a comment. This revision is now accepted and ready to land. Looks good to me but I definitely want to hear what @efriedma has to say. Repository: rL LLVM https://reviews.llvm.org/D34523 ___ cfe-comm

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

majnemer added inline comments. Comment at: lib/AST/MicrosoftMangle.cpp:993-994 + Out << "YAX"; + // struct __block_literal * + Out << "PA"; + mangleArtificalTagType(TTK_Struct, compnerd wrote: > majnemer wrote: > > Shouldn't we also mangle an

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

majnemer added a comment. We need tests that show that it does the right thing in blocks defined in classes in classes and other nested concepts. Comment at: lib/AST/MicrosoftMangle.cpp:980-981 + unsigned Discriminator = BD->getBlockManglingNumber(); + if (!Discrimin

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

majnemer added inline comments. Comment at: lib/AST/MicrosoftMangle.cpp:985 + + Out << '?' << Discriminate("_block_invoke", Discriminator) << '@'; + if (const auto *RD = dyn_cast(DC)) Should this be `Out << '?' << mangleSourceName(Discriminate("_block_i

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

majnemer added inline comments. Comment at: include/__config:234-235 +// a MS compatibility version is specified. # ifndef __MINGW32__ -#define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library +#ifdef _MSC_VER +# define _LIBCPP_MSVCRT // Using Microsoft's C Ru

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

majnemer added inline comments. Comment at: lib/AST/MicrosoftMangle.cpp:981-984 + Out << "YAXPAU__block_literal"; + if (Discriminator) +Out<< '_' << Discriminator; + Out << "@@@Z"; compnerd wrote: > majnemer wrote: > > I think you want to u

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

majnemer added inline comments. Comment at: lib/AST/MicrosoftMangle.cpp:981-984 + Out << "YAXPAU__block_literal"; + if (Discriminator) +Out<< '_' << Discriminator; + Out << "@@@Z"; I think you want to use mangleArtificalTagType here. Repo

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

majnemer added a comment. Can you please have a test where you define blocks w/ static variables in several default arguments of the same function? Also would be good to have this in NSDMIs in class definitions. Repository: rL LLVM https://reviews.llvm.org/D34523

[PATCH] D34377: Support MS builtins using 'long' on Darwin/LP64

majnemer added inline comments. Comment at: lib/AST/ASTContext.cpp:8551 + break; +} case 'W': bruno wrote: > bruno wrote: > > rnk wrote: > > > compnerd wrote: > > > > I agree with @majnemer. Why not base this on the Int64Type? > > > I'd suggest thi

[PATCH] D32428: Fix for Itanium mangler issue with templates (bug: 31405)

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

[PATCH] D33852: Enable __declspec(selectany) on linux

majnemer added inline comments. Comment at: include/clang/Basic/Attr.td:2421 -def SelectAny : InheritableAttr, TargetSpecificAttr { +def SelectAny : InheritableAttr, TargetSpecificAttr { let Spellings = [Declspec<"selectany">, GCC<"selectany">]; selectany sh

  1   2   >