[PATCH] D79895: Add a new warning to warn when passing uninitialized variables as const reference parameters to a function

rnk added inline comments. Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:1518 UsesMap uses; + UsesMap constRefUses; If possible, it would be nice to avoid having a second map. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79895/new/ https:

[PATCH] D80172: Revert "Re-fix _lrotl/_lrotr to always take Long, no matter the platform."

rnk added a comment. I'd be fine going back to the behavior from before. > Without this behavior there is no intrinsic for 32 bit rotates on these > platforms. Strictly speaking, there seems to be `__builtin_rotate(left|right)32`, which is portable to all clang platforms. Repository: rG LL

[PATCH] D40508: Replace long type names in IR with hashes

rnk added a comment. It's not clear to me that this abbreviation functionality should live in Support. Clang probably wants enough control over this (assuming we're doing it) that the logic should live in clang. I also think we might want to try solving this a completely different way: just do

[PATCH] D39673: Toolchain: Normalize dwarf, sjlj and seh eh

rnk requested changes to this revision. rnk added inline comments. This revision now requires changes to proceed. Comment at: lib/Driver/ToolChain.cpp:450-458 + const llvm::Target *TT = llvm::TargetRegistry::lookupTarget(TS, Error); + if (!TT) +return llvm::ExceptionHandlin

[PATCH] D40621: MS ABI: Treat explicit instantiation definitions of dllimport function templates as explicit instantiation decls (PR35435)

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Looks good, thanks! https://reviews.llvm.org/D40621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listi

[PATCH] D40660: Enable auto-linking on Windows

rnk added inline comments. Comment at: include/__config:1266 +#if defined(_LIBCPP_ABI_MICROSOFT) +# if defined(_DLL) && !defined(_LIBCPP_BUILDING_LIBRARY) +# if defined(_LIBCPP_DEBUG) smeenai wrote: > This feels more like a Windows (or perhaps COFF) thing than

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

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

[PATCH] D41036: IRGen: When performing CFI checks, load vtable pointer from vbase when necessary.

rnk added a comment. Looks good! Repository: rL LLVM https://reviews.llvm.org/D41036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132810: [clang][MinGW] Add `-mguard=cf` and `-mguard=cf-nochecks`

rnk added a comment. +@maskray, should we use `-mguard=` as the GCC spelling of the MSVC `/GUARD:` flag? Short flags are nice, but it seems there may be a risk of GCC using `-mguard=` for something else at some point. I lean towards using `-mguard=` as is. Repository: rG LLVM Github Monorep

[PATCH] D132810: [clang][MinGW] Add `-mguard=cf` and `-mguard=cf-nochecks`

rnk added a comment. (I added @maskray based on the new code ownership structure proposed here: https://discourse.llvm.org/t/rfc-proposed-changes-to-clangs-code-ownership/64813) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132810/new/ https://rev

[PATCH] D132810: [clang][MinGW] Add `-mguard=cf` and `-mguard=cf-nochecks`

rnk accepted this revision. rnk 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/D132810/new/ https://reviews.llvm.org/D132810 ___ cfe-c

[PATCH] D133711: [Sema] Reject array element types whose alignments are larger than their sizes

rnk added a comment. Usually the solution is to round up the size to the alignment. Can you provide godbolt examples of what GCC and MSVC do? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133711/new/ https://reviews.llvm.org/D133711 _

[PATCH] D133711: [Sema] Reject array element types whose alignments are larger than their sizes

rnk added a comment. I agree, there's no reason to change the i686 MSVC record layout algorithm. It was written, tested, done, it's fragile. We used to have some decent continuous integration testing for ABI issues like this, but as usual these things require care and feeding and it did not sur

[PATCH] D133817: MSVC ABI: Looks like even non-aarch64 uses the MSVC/14 definition for pod/aggregate passing

rnk added a comment. Thanks! > But I had > trouble figuring out how to exercise this functionality properly to add > test coverage and then compare that to MSVC itself... - I got very > confused/turned around trying to test this, so I've given up enough to > send what I have out for review, but h

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

rnk added a comment. This seems like a new error that will be pretty easy to run into, especially in the funky MSVC virtual base case. @aaron.ballman proposed some kind of new breaking change announcement mechanism. Can you add a release note and send an announcement following that process? I t

[PATCH] D133920: [X86][fastcall] Move capability check before free register update

rnk added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:1781 + + return (Ty->isIntegralOrEnumerationType() || Ty->isPointerType() || + Ty->isReferenceType()); I think we could improve readability here with some named variable boo

[PATCH] D122087: Add HLSL Language Option and Preprocessor

rnk accepted this revision. rnk added a comment. Herald added a subscriber: StephenFan. lgtm Comment at: clang/lib/Frontend/InitPreprocessor.cpp:401-403 +uint32_t StageInteger = StageInteger = +(uint32_t)TI.getTriple().getEnvironment() - +(uint32_t)llvm::Tri

[PATCH] D121733: Clean pathnames in FileManager.

rnk added a comment. Ignoring the ".." path components for the moment, is this patch good to go as is? It doesn't affect that behavior. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121733/new/ https://reviews.llvm.org/D121733 ___

[PATCH] D122627: [HLSL] Fix MSFT Attribute parsing, add numthreads

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang/include/clang/Basic/AttrDocs.td:6377 +are required. The ``X``, ``Y``, and ``Z`` values provided to the attribute +dictate the thread id. Total number of thre

[PATCH] D130709: MSVC compatibility mode: fix error on unqualified templated base class initialization in case of partial specialization

rnk added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:4316 + for (auto const &Base : ClassDecl->bases()) { +QualType BT = cast(Base.getType())->getNamedType(); +const auto *BaseTemplate = dyn_cast(BT); Is this cas

[PATCH] D130123: Extend ptr32 support to be applied on typedef

rnk added a comment. That sounds reasonable to me, I confirmed that MSVC really only lets you apply these attributes directly to pointer types and to typedefs. Can you add a test for the other most common type sugar node, the template parameter? It looks like this: template void f(T __ptr

[PATCH] D130123: Extend ptr32 support to be applied on typedef

rnk added a comment. In D130123#3696752 , @Ariel-Burton wrote: > What is your expectation for your template code fragment? MSVC does not > accept it. Yes, I checked, MSVC rejects it, so clang should have test expectations to confirm that. It seems in

[PATCH] D130123: Extend ptr32 support to be applied on typedef

rnk added a comment. In D130123#3698399 , @Ariel-Burton wrote: > Just to make sure that we're on the same page, you'd like to see a test that > confirms that clang is rejecting the `template void f(T __ptr32 > a)` example, and possibly one that check

[PATCH] D130123: Extend ptr32 support to be applied on typedef

rnk accepted this revision. rnk added a comment. lgtm, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130123/new/ https://reviews.llvm.org/D130123 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D126676: [clang] Disallow differences in defines used for creating and using PCH

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. I don't have any concerns, this seems reasonable to me. Since Jan offered to test it, please wait to hear back before landing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D130123: Extend ptr32 support to be applied on typedef

rnk accepted this revision. rnk added a comment. In D130123#3708309 , @Ariel-Burton wrote: > - Add case to deal with ElaboratedTypes. Let's still stick with this code, but I at least feel justified raising the concern that perhaps we should look throug

[PATCH] D104058: ThinLTO: Fix inline assembly references to static functions with CFI

rnk added inline comments. Comment at: llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp:77 + // references from inline assembly. + std::string Alias = ".set \"" + OldName + "\",\"" + NewName + "\"\n"; + ExportM.appendModuleInlineAsm(Alias); nickdes

[PATCH] D106585: Fix clang debug info irgen of i128 enums

rnk created this revision. rnk added reviewers: mizvekov, dblaikie, MaskRay. Herald added subscribers: dexonsmith, hiraditya. rnk requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: llvm-commits. DIEnumerator stores an APInt as of April 2020, so now we

[PATCH] D105320: [CodeView] Saturate values bigger than supported by APInt.

rnk added subscribers: dblaikie, MaskRay, LemonBoy. rnk added a comment. I decided it would be faster and more convenient to respond in the form of a code review for clang, so here it is: D106585 Assuming that goes through, go ahead and rebase onto that after i

[PATCH] D106585: Fix clang debug info irgen of i128 enums

rnk added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3117 +llvm::APSInt Value = Enum->getInitVal(); +Value.setIsSigned(IsSigned); +Enumerators.push_back(DBuilder.createEnumerator(Enum->getName(), Value)); dblaikie wrote: > Is the v

[PATCH] D106585: Fix clang debug info irgen of i128 enums

rnk updated this revision to Diff 360978. rnk added a comment. - move Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106585/new/ https://reviews.llvm.org/D106585 Files: clang/lib/CodeGen/CGDebugInfo.cpp clang/test/CodeGenCXX/debug-info-enum-i12

[PATCH] D106585: Fix clang debug info irgen of i128 enums

rnk added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3117 +llvm::APSInt Value = Enum->getInitVal(); +Value.setIsSigned(IsSigned); +Enumerators.push_back(DBuilder.createEnumerator(Enum->getName(), Value)); dblaikie wrote: > rnk wrot

[PATCH] D106585: Fix clang debug info irgen of i128 enums

rnk added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3117 +llvm::APSInt Value = Enum->getInitVal(); +Value.setIsSigned(IsSigned); +Enumerators.push_back(DBuilder.createEnumerator(Enum->getName(), Value)); rnk wrote: > dblaikie wrot

[PATCH] D106585: Fix clang debug info irgen of i128 enums

rnk updated this revision to Diff 361306. rnk added a comment. - add comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106585/new/ https://reviews.llvm.org/D106585 Files: clang/lib/CodeGen/CGDebugInfo.cpp clang/test/CodeGenCXX/debug-info-

[PATCH] D106585: Fix clang debug info irgen of i128 enums

rnk added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3117 +llvm::APSInt Value = Enum->getInitVal(); +Value.setIsSigned(IsSigned); +Enumerators.push_back(DBuilder.createEnumerator(Enum->getName(), Value)); MaskRay wrote: > dblaikie

[PATCH] D106721: [AArch64] Implemnt MSVC __mulh and __umulh builtins and corresponding IR level intrinsics

rnk added a comment. In D106721#2901728 , @efriedma wrote: > Do we need LLVM intrinsics for these? For the x86 equivalents, we just > generate `mul i128`. I worry that LLVM will end up generating a call to compiler-rt to implement i128 arithmetic, esp

[PATCH] D106791: [clang-cl] Expose -fmodules and related flags in the driver (PR43391)

rnk accepted this revision. rnk 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/D106791/new/ https://reviews.llvm.org/D106791 ___ cfe-c

[PATCH] D106790: prfchwintrin.h: Make _m_prefetchw take a pointer to volatile (PR49124)

rnk added a comment. Does this match GCC? They also provide this Intel intrinsic. Seems reasonable in principle, though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106790/new/ https://reviews.llvm.org/D106790 __

[PATCH] D106721: [AArch64] Implemnt MSVC __mulh and __umulh builtins and corresponding IR level intrinsics

rnk added a comment. In D106721#2902586 , @efriedma wrote: > We won't call compiler-rt for i128 multiply on aarch64. It's not worthwhile > under any circumstances. > > Pattern-matching 64/64->128 multiply in SelectionDAG legalization has been > reliabl

[PATCH] D106585: Fix clang debug info irgen of i128 enums

This revision was automatically updated to reflect the committed changes. Closed by commit rG323049329939: Fix clang debug info irgen of i128 enums (authored by rnk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106585/new/ https://reviews.llvm.org

[PATCH] D105320: [CodeView] Saturate values bigger than supported by APInt.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm with minor comment adjustments Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:3167 + + // TODO: Need to support bigger ints like __int128. + OS.AddComment("Value"

[PATCH] D105320: [CodeView] Saturate values bigger than supported by APInt.

rnk added a comment. Yes, I filed https://bugs.llvm.org/show_bug.cgi?id=51221 for it. I plan to disable the test under asan for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105320/new/ https://reviews.llvm.org/D105320 __

[PATCH] D106585: Fix clang debug info irgen of i128 enums

rnk added a comment. In D106585#2905663 , @dblaikie wrote: > In D106585#2902588 , @dblaikie > wrote: > >> Preserving existing behavior sounds OK - maybe some comment about that it >> might be good to remove so t

[PATCH] D106790: prfchwintrin.h: Make _m_prefetchw take a pointer to volatile (PR49124)

rnk added a comment. In D106790#2906762 , @hans wrote: > In D106790#2905110 , @rnk wrote: > >> Does this match GCC? They also provide this Intel intrinsic. > > No, they define it without volatile (and also without

[PATCH] D106925: COFF/ELF: Place llvm.global_ctors elements in llvm.used if comdat is used

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm So the main impact here is that, on ELF, linker GC will no longer be able to GC vague linkage global variables with dynamic initializers. Those are things like - C++17 inline globals - selecta

[PATCH] D106790: prfchwintrin.h: Make _m_prefetchw take a pointer to volatile (PR49124)

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. In D106790#2909567 , @hans wrote: > I don't think that should cause any problems. Passing a less qualified > pointer to a more cv-qualified parameter should

[PATCH] D120936: [Sema][Windows] Don't special-case void* in __unaligned conversions.

rnk added a comment. I remember writing up feedback on this patch, but it's gone now, oh well. I wasn't able to find any history for why `void*` was special here. I see that MSVC doesn't warn when converting from `__unaligned int*` to `int*`, but that seems dangerous. Our team has an active fea

[PATCH] D120936: [Sema][Windows] Don't special-case void* in __unaligned conversions.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120936/new/ https://reviews.llvm.org/D120936

[PATCH] D121412: Complete the list of single-underscore keywords for MSVC compat.

rnk added a subscriber: rsmith. rnk added a comment. @rsmith has previously advocated for a policy of only permitting conforming language extensions under fms-extensions. These single underscore names are not in the implementors namespace, so he has argued they should be under fms-compatibility

[PATCH] D121412: Complete the list of single-underscore keywords for MSVC compat.

rnk accepted this revision. rnk 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/D121412/new/ https://reviews.llvm.org/D121412 ___ cfe-c

[PATCH] D121497: Lex: add support for `{,u}i128` Microsoft extension

rnk added a comment. Seems reasonable, this could use far more testing. Comment at: clang/test/Lexer/ms-extensions.c:27 +#define INT128_MAX 170141183460469231731687303715884105727i128 +#define UINT128_MAX 0xui128 + This seems li

[PATCH] D121865: [CodeGen] Inline _byteswap_* builtins.

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

[PATCH] D121733: Clean pathnames in FileManager.

rnk added a comment. I've been somewhat afraid to touch this code because of symlinks. Is that something we need to worry about? Consider this path: root/symlink/../foo.h. remove_dots will turn this into root/foo.h, but if symlink points to some unrelated directory, the .. directory entry point

[PATCH] D121687: [clang-tidy] Don't try to build CTTestTidyModule for Windows with dylibs

rnk accepted this revision. rnk 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/D121687/new/ https://reviews.llvm.org/D121687 ___ cfe-c

[PATCH] D123405: [dllexport] odr-use constexpr default args for constructor closures

rnk added a comment. Looks good to me after the fact. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123405/new/ https://reviews.llvm.org/D123405 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D123345: Treat `std::move`, `forward`, and `move_if_noexcept` as builtins.

rnk added a comment. Generally speaking, this sounds like a good idea to me. One time in 2019 I used -ftime-trace+ClangBuildAnalyzer and it told me that std::unique_ptr was the most expensive template because it is instantiated so m

[PATCH] D122734: [CUDA][HIP] Fix mangling number for local struct

rnk added inline comments. Herald added a subscriber: mattd. Comment at: clang/include/clang/AST/ASTContext.h:682 +/// Current name mangling is for device name in host compilation. +bool MangleDeviceNameInHostCompilation = false; + } CUDANameMangleCtx; I

[PATCH] D124032: [COFF, ARM64] Add __break intrinsic

rnk accepted this revision. rnk 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/D124032/new/ https://reviews.llvm.org/D124032 ___ cfe-c

[PATCH] D118428: [clang-cl] Support the /JMC flag

rnk added inline comments. Comment at: llvm/lib/CodeGen/JMCInstrumenter.cpp:145 + LLVMContext &Ctx = M.getContext(); + bool UseX86FastCall = Triple(M.getTargetTriple()).getArch() == Triple::x86; + ychen wrote: > ychen wrote: > > hans wrote: > > > I still worry

[PATCH] D118428: [clang-cl] Support the /JMC flag

rnk added a comment. In D118428#3294411 , @ychen wrote: > The passes in `lib/Transforms/Instrumentation` runs with > `EmitAssemblyHelper::RunOptimizationPipeline`. JMC pass runs with > `EmitAssemblyHelper::RunCodegenPipeline`. Sure, but that's a choice

[PATCH] D118428: [clang-cl] Support the /JMC flag

rnk added a comment. In D118428#3294935 , @ychen wrote: > The instrumentation is per-function, ideally for each function that has > debuginfo and ends up in the executable. So I want it to happen the last in > the IR codegen pipeline (target could arran

[PATCH] D118744: [clang] Fix some clang->llvm type cache invalidation issues

rnk added a comment. This seems unfortunately complex, but I think we can live with it for a year or two. Is it possible to use the compile time tracker to benchmark if this clang->LLVM type cache actually saves compile time? This change disables the cache pretty often, and I'm wondering if "p

[PATCH] D119051: Fix pod-packed functionality to use the C++11 definition of pod-ness

rnk added a comment. Looks like the test fails on the Windows pre-merge bot: https://buildkite.com/llvm-project/premerge-checks/builds/77696#1836f181-a998-4695-b587-a83239ea The debian bot seems to be failing for unrelated (clang tooling) reasons. Comment at: clang/lib/AS

[PATCH] D84225: [CFE] Add nomerge function attribute to inline assembly.

rnk added a comment. I think LLVM already doesn't do some tail merging optimizations on inline asm, but allowing the use of the attribute is more principled, and will block more optimizations (CSE). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84

[PATCH] D118744: [clang] Fix some clang->llvm type cache invalidation issues

rnk accepted this revision. rnk 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/D118744/new/ https://reviews.llvm.org/D118744 ___ cfe-c

[PATCH] D119234: [clang] [MinGW] Recognize -lcrtdll as a library replacing -lmsvcrt

rnk accepted this revision. rnk 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/D119234/new/ https://reviews.llvm.org/D119234 ___ cfe-c

[PATCH] D84225: [CFE] Add nomerge function attribute to inline assembly.

rnk added a comment. In D84225#3304189 , @pengfei wrote: > It's not a workaround. We do need to avoid the merging sometime. For example, > given we have 2 branches begin with inline asm of `endbr`. We have to use > `nomerge` to stop them been merged out

[PATCH] D119215: [clang] Properly cache member pointer LLVM types

rnk added a comment. Seems reasonable Comment at: clang/lib/CodeGen/CodeGenTypes.cpp:776 + auto *C = MPTy->getClass(); + auto I = RecordsWithOpaqueMemberPointers.find(C); + if (I == RecordsWithOpaqueMemberPointers.end()) { Can this use something

[PATCH] D119215: [clang] Properly cache member pointer LLVM types

rnk accepted this revision. rnk 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/D119215/new/ https://reviews.llvm.org/D119215 ___ cfe-c

[PATCH] D116861: [UBSan] Fix incorrect alignment reported when global new returns an offset pointer

rnk added inline comments. Comment at: clang/lib/CodeGen/CGExprCXX.cpp:1658-1659 Target.getNewAlign(), getContext().getTypeSize(allocType))); allocationAlign = std::max( allocationAlign, getContext().toCharUnitsFromBits(AllocatorAlign)); } --

[PATCH] D119711: Add asan support for MSVC debug runtimes

rnk added subscribers: cbezault, vitalybuka. rnk added a comment. I'm hesitant to do this. The main reason we disabled the use of the debug runtimes was that the debug runtimes interfered with malloc interception, which leads to crashes and a generally poor user experience. I'd like some confir

[PATCH] D120272: [CUDA] Add driver support for compiling CUDA with the new driver

rnk added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6223-6224 if (IsCuda || IsHIP) { -if (Args.hasFlag(options::OPT_fgpu_rdc, options::OPT_fno_gpu_rdc, false)) +if (Args.hasFlag(options::OPT_fgpu_rdc, options::OPT_fno_gpu_rdc, false) || +

[PATCH] D124435: [X86] Always extend the integer parameters in callee

rnk added inline comments. Comment at: clang/include/clang/CodeGen/CGFunctionInfo.h:120 bool SRetAfterThis : 1; // isIndirect() bool InReg : 1; // isDirect() || isExtend() || isIndirect() bool CanBeFlattened: 1; // isDirect() Instead of int

[PATCH] D124613: In MSVC compatibility mode, friend function declarations behave as function declarations

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm I plugged this into godbolt to confirm the behavior of MSVC: https://gcc.godbolt.org/z/v3P3WbxG3 This incompatibility has existed for a long time, right? Repository: rG LLVM Github Monorepo

[PATCH] D122734: [CUDA][HIP] Fix mangling number for local struct

rnk added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:682 +/// Current name mangling is for device name in host compilation. +bool MangleDeviceNameInHostCompilation = false; + } CUDANameMangleCtx; yaxunl wrote: > rnk wrote: > > It d

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

rnk added a comment. Thanks! In D124762#3485478 , @sgraenitz wrote: > I guess testing must be split in two: > > - Clang wants to make sure the "funclet" bundle operand gets emitted for ObjC > ARC runtime calls on Windows. Maybe that fits into > clang/t

[PATCH] D124666: In MSVC compatibility mode, handle unqualified templated base class initialization

rnk requested changes to this revision. rnk added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:4310-4311 // If no results were found, try to correct typos. TypoCorrection Corr; MemInitializerVali

[PATCH] D124842: [NFC][CUDA][HIP] rework mangling number for aux target

rnk accepted this revision. rnk added a comment. lgtm, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124842/new/ https://reviews.llvm.org/D124842 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/m

[PATCH] D124666: In MSVC compatibility mode, handle unqualified templated base class initialization

rnk added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:4310-4311 // If no results were found, try to correct typos. TypoCorrection Corr; MemInitializerValidatorCCC CCC(ClassDecl); rnk wrote: > We have to make sure our MS comp

[PATCH] D124666: In MSVC compatibility mode, handle unqualified templated base class initialization

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, thanks! In D124666#3490871 , @frederic-tingaud-sonarsource wrote: > By the way, in the current state of the code, there is no risk of such > conflic

[PATCH] D126676: [clang] Disallow differences in defines used for creating and using PCH

rnk added subscribers: arphaman, dexonsmith. rnk added a comment. Seeking additional opinions on the PCH change: @dexonsmith @arphaman The use case makes sense to me. It seems reasonable to me that the compiler shouldn't use the same PCH file with mismatched defines on the command line. =

[PATCH] D130123: Extend ptr32 support to be applied on typedef

rnk added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7116 + for (;;) { +if (const TypedefType *TT = dyn_cast(Desugared)) { + Desugared = TT->desugar(); This seems like a good place to use getSingleStepDesugaredType to look through all typ

[PATCH] D130123: Extend ptr32 support to be applied on typedef

rnk added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7116 + for (;;) { +if (const TypedefType *TT = dyn_cast(Desugared)) { + Desugared = TT->desugar(); Ariel-Burton wrote: > rnk wrote: > > This seems like a good place to use getSingleStepD

[PATCH] D130303: Handle template parameter-dependent bit field widths in libclang

rnk added a comment. Dmitri, do you know a good libclang point of contact for the new API? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130303/new/ https://reviews.llvm.org/D130303 ___ cfe-commits maili

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

rnk added a comment. This seems reasonable, but I worry about the consequences of creating lots of unnamed global variables. What will gdb do with so many unnamed globals? What will the PDB linker do with all these unnamed globals? I can't answer these questions, and you're welcome to try and f

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

rnk added a comment. In D123534#3507891 , @hctim wrote: > Not sure about PDB. I did run a quick test with `gdb`, and very > unscientifically, didn't notice any difference in usability or errors between > pre- and post-this patch on a `clang` invocation

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Looks good to me. I generally agree with Adrian's suggestions. Comment at: llvm/test/DebugInfo/COFF/global-no-strings.ll:1 +; RUN: llc < %s | FileCheck %s +; RUN: llc < %s -filetyp

[PATCH] D124613: In MSVC compatibility mode, friend function declarations behave as function declarations

rnk added a comment. I haven't given any input yet because I haven't yet seen a reduced example of the conforming code that is broken by this change. If someone can put together a small godbolt example that shows the issue affecting ADL, I'd appreciate it. I don't fully understand the code patt

[PATCH] D126023: [MSVC, ARM64] Add __writex18 intrinsics

rnk added a comment. I was unable to find any documentation for the meaning of AArch64 addrspace(256), and I wasn't able to figure it out after studying the code in llvm/lib/Target/AArch64 for ten minutes or so. The change seems fine, but please add some documentation as a follow-up. X86 has so

[PATCH] D126224: Add DWARF string debug to clang release notes.

rnk added a comment. I am reminded of the perennial problem of "optional" protobuf fields that, when omitted, will cause production crashes. Do you think it would be less disruptive to synthesize a name? I believe the string lives in a string pool, so naming all string literals `` seems like i

[PATCH] D66606: IR. Change strip* family of functions to not look through aliases.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm GlobalOpt already replaces replaceable aliases, and if it doesn't do it enough, we can adjust the pass pipeline to fix the phase ordering problem. Repository: rG LLVM Github Monorepo CHANG

[PATCH] D66770: Move EH spec mismatches under -fms-compatibility

rnk created this revision. rnk added a reviewer: hans. Herald added a project: clang. -fms-extensions is intended to enable conforming language extensions and -fms-compatibility is intended to language rule relaxations, so a user could plausibly compile with -fno-ms-compatibility on Windows while

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. I think we're ready to proceed here, lgtm. Shout if I've misrepresented anything. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64931/new/ https:/

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

rnk added a comment. I plan to take a look at this tomorrow, sorry for putting this off for a while. :( Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65761/new/ https://reviews.llvm.org/D65761 ___ cfe

[PATCH] D66813: Remove clang-tidy-vs plugin from clang-tools-extra

rnk accepted this revision. rnk 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/D66813/new/ https://reviews.llvm.org/D66813 ___ cfe-c

[PATCH] D66770: Move EH spec mismatches under -fms-compatibility

This revision was automatically updated to reflect the committed changes. Closed by commit rG39aa8954a484: Move EH spec mismatches under -fms-compatibility (authored by rnk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66770/new/ https://reviews.l

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

rnk added a comment. This change needs a clang CodeGen test to show that we generate IR with `__ptr32` / `__ptr64` in the correct places. We should already have some semantic tests, but we may need more. Comment at: clang/lib/AST/MicrosoftMangle.cpp:1874 +case LangAS::ptr

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

rnk added a comment. I'm not sure what happens, but I see you added .gitattributes. I'd commit it as is. Buildbots using svn will keep working. You can check that the monorepo has the right line endings afterwards, and try again if not. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D665

[PATCH] D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial

rnk added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:3581 +llvm::DILocalScope *PrevScope = +!LexicalBlockStack.empty() +? dyn_cast(LexicalBlockStack.back()) Is it OK to look up the lexical block stack at this point? The block

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk created this revision. rnk added reviewers: rsmith, hans, efriedma. Herald added a subscriber: fedor.sergeev. Herald added a project: clang. This avoids cloning variadic virtual methods when the target supports musttail and the return type is not covariant. I think we never implemented this pr

<    4   5   6   7   8   9   10   11   12   13   >