[PATCH] D31534: [ThinLTO] Handle -emit-llvm* in ThinLTO backends

pcc added a comment. Why not call `Module::print()` or `WriteBitcodeToFile` directly instead of creating a pass manager? https://reviews.llvm.org/D31534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/l

[PATCH] D31534: [ThinLTO] Handle -emit-llvm* in ThinLTO backends

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

[PATCH] D32064: [asan] Disable ASan global-GC depending on the target and CGOpts

pcc added a comment. I think it would be better to move this logic to the driver and have it pass in an `-mllvm` flag. The sanitizer passes should really be taking no arguments in the constructor like the other passes, so I don't want us to add another argument here. Repository: rL LLVM ht

[PATCH] D32043: [Driver] Load all necessary default sanitizer blacklists

pcc added a comment. This seems reasonable to me, although it's unfortunate that the design of the sanitizer blacklist feature does not (at present) allow different blacklists for different sanitizers. @eugenis what do you think? https://reviews.llvm.org/D32043

[PATCH] D32064: [asan] Disable ASan global-GC depending on the target and compiler flags

pcc added a comment. In https://reviews.llvm.org/D32064#728629, @rnk wrote: > In https://reviews.llvm.org/D32064#726861, @pcc wrote: > > > I think it would be better to move this logic to the driver and have it > > pass in an `-mllvm` flag. The sanitizer passes should really be taking no > > ar

[PATCH] D28843: IRGen: Start using the WriteThinLTOBitcode pass.

pcc created this revision. This is the final change necessary to support CFI with ThinLTO. TODO: avoid breaking Darwin. Depends on https://reviews.llvm.org/D28840 https://reviews.llvm.org/D28843 Files: clang/lib/CodeGen/BackendUtil.cpp clang/test/CodeGenCXX/type-metadata-thinlto.cpp Inde

[PATCH] D28843: IRGen: Start using the WriteThinLTOBitcode pass.

pcc added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:694 +else + PerModulePasses.add( + createBitcodeWriterPass(*OS, CodeGenOpts.EmitLLVMUseLists)); tejohnson wrote: > Can we transform other callers of createBitcodeWriterPass

[PATCH] D28843: IRGen: Start using the WriteThinLTOBitcode pass.

pcc added inline comments. Comment at: clang/test/CodeGenCXX/type-metadata-thinlto.cpp:2 +// RUN: %clang_cc1 -flto=thin -triple x86_64-unknown-linux -fvisibility hidden -emit-llvm-bc -o %t %s +// RUN: llvm-modextract -o - -n 1 %t | llvm-dis | FileCheck %s + mehd

[PATCH] D28877: Move vtable type metadata emission behind a cc1-level flag.

pcc created this revision. In ThinLTO mode, type metadata will require the module to be written as a multi-module bitcode file, which is currently incompatible with the Darwin linker. It is also useful to be able to enable or disable multi-module bitcode for testing purposes. This introduces a cc1

[PATCH] D28843: IRGen: Start using the WriteThinLTOBitcode pass.

pcc added inline comments. Comment at: clang/test/CodeGenCXX/type-metadata-thinlto.cpp:2 +// RUN: %clang_cc1 -flto=thin -triple x86_64-unknown-linux -fvisibility hidden -emit-llvm-bc -o %t %s +// RUN: llvm-modextract -o - -n 1 %t | llvm-dis | FileCheck %s + tejo

[PATCH] D28877: Move vtable type metadata emission behind a cc1-level flag.

This revision was automatically updated to reflect the committed changes. Closed by commit rL292448: Move vtable type metadata emission behind a cc1-level flag. (authored by pcc). Changed prior to commit: https://reviews.llvm.org/D28877?vs=84900&id=84902#toc Repository: rL LLVM https://revi

[PATCH] D28843: IRGen: Start using the WriteThinLTOBitcode pass.

pcc updated this revision to Diff 84903. pcc added a comment. Refresh https://reviews.llvm.org/D28843 Files: clang/lib/CodeGen/BackendUtil.cpp clang/test/CodeGenCXX/type-metadata-thinlto.cpp Index: clang/test/CodeGenCXX/type-metadata-thinlto.cpp ===

[PATCH] D28843: IRGen: Start using the WriteThinLTOBitcode pass.

pcc updated this revision to Diff 84907. pcc added a comment. Herald added a subscriber: mgorny. - Add missing test dependency https://reviews.llvm.org/D28843 Files: clang/lib/CodeGen/BackendUtil.cpp clang/test/CMakeLists.txt clang/test/CodeGenCXX/type-metadata-thinlto.cpp Index: clang/

[PATCH] D28843: IRGen: Start using the WriteThinLTOBitcode pass.

This revision was automatically updated to reflect the committed changes. Closed by commit rL292662: IRGen: Start using the WriteThinLTOBitcode pass. (authored by pcc). Changed prior to commit: https://reviews.llvm.org/D28843?vs=84907&id=85198#toc Repository: rL LLVM https://reviews.llvm.or

[PATCH] D29067: IRGen: When loading the main module in the distributed ThinLTO backend, look for the module containing the summary.

pcc created this revision. Herald added a subscriber: mgorny. https://reviews.llvm.org/D29067 Files: clang/include/clang/CodeGen/BackendUtil.h clang/include/clang/CodeGen/CodeGenAction.h clang/lib/CodeGen/BackendUtil.cpp clang/lib/CodeGen/CodeGenAction.cpp clang/test/CMakeLists.txt cl

[PATCH] D29067: IRGen: When loading the main module in the distributed ThinLTO backend, look for the module containing the summary.

pcc updated this revision to Diff 85618. pcc added a comment. Address review comments https://reviews.llvm.org/D29067 Files: clang/lib/CodeGen/CodeGenAction.cpp clang/test/CMakeLists.txt clang/test/CodeGen/thinlto-multi-module.ll Index: clang/test/CodeGen/thinlto-multi-module.ll ===

[PATCH] D29067: IRGen: When loading the main module in the distributed ThinLTO backend, look for the module containing the summary.

pcc added inline comments. Comment at: clang/include/clang/CodeGen/BackendUtil.h:51 + llvm::Expected + FindThinLTOModule(llvm::MemoryBufferRef MBRef); } mehdi_amini wrote: > Indentation seems strange? Yes, it's what clang-format does though.

[PATCH] D29067: IRGen: When loading the main module in the distributed ThinLTO backend, look for the module containing the summary.

This revision was automatically updated to reflect the committed changes. Closed by commit rL293209: IRGen: When loading the main module in the distributed ThinLTO backend, look… (authored by pcc). Changed prior to commit: https://reviews.llvm.org/D29067?vs=85618&id=85958#toc Repository: rL

[PATCH] D29545: Driver: Do not link safestack with --whole-archive.

pcc created this revision. This allows it to be used with the other sanitizers. https://reviews.llvm.org/D29545 Files: clang/lib/Driver/Tools.cpp clang/test/Driver/sanitizer-ld.c Index: clang/test/Driver/sanitizer-ld.c === --

[PATCH] D29545: Driver: Do not link safestack with --whole-archive.

This revision was automatically updated to reflect the committed changes. Closed by commit rL294274: Driver: Do not link safestack with --whole-archive. (authored by pcc). Changed prior to commit: https://reviews.llvm.org/D29545?vs=87106&id=87358#toc Repository: rL LLVM https://reviews.llvm

[PATCH] D34955: [Basic] Detect Git submodule version in CMake

pcc added a comment. FYI, I don't think `HEAD` on its own will work because it is not necessarily updated on every commit, see discussion on https://reviews.llvm.org/D31985. https://reviews.llvm.org/D34955 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D42146: libcxx: Disable CFI in function std::get_temporary_buffer.

This revision was automatically updated to reflect the committed changes. Closed by commit rCXX322744: libcxx: Disable CFI in function std::get_temporary_buffer. (authored by pcc, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D42146?vs

[PATCH] D42225: libcxx: Provide overloads for basic_filebuf::open() et al that take wchar_t* filenames on Windows.

This revision was automatically updated to reflect the committed changes. Closed by commit rCXX323170: libcxx: Provide overloads for basic_filebuf::open() et al that take wchar_t*… (authored by pcc, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://review

[PATCH] D42220: libcxx: Use vcruntime declarations for typeinfo on Windows.

This revision was automatically updated to reflect the committed changes. Closed by commit rCXX323491: libcxx: Use vcruntime declarations for typeinfo on Windows. (authored by pcc, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D42220?v

[PATCH] D42680: [ThinLTO] Ignore -fthinlto-index= for non-ThinLTO files

pcc requested changes to this revision. pcc added a comment. This revision now requires changes to proceed. This doesn't seem right to me. In a mixed full/thin LTO link the full LTO module would be compiled during the indexing phase. We don't want to compile it again in the backend as it could l

[PATCH] D42725: IRGen: Move vtable load after argument evaluation.

pcc created this revision. pcc added reviewers: vlad.tsyrklevich, rsmith. This change reduces the live range of the loaded function pointer, resulting in a slight code size decrease (~10KB in clang), and also improves the security of CFI for virtual calls by making it less likely that the function

[PATCH] D30239: enable -flto=thin, -flto-jobs=, and -fthinlto-index= in clang-cl

pcc added subscribers: cfe-commits, pcc. pcc added a comment. Can you please add a ThinLTO test to lld before adding driver support? https://reviews.llvm.org/D30239 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bi

[PATCH] D30920: Do not pass -Os and -Oz to the Gold plugin

pcc added a comment. In https://reviews.llvm.org/D30920#700077, @tejohnson wrote: > Until everything is converted to using size attributes, it seems like a > correct fix for the bug is to accept these options in the gold-plugin and > pass through the LTO API to the PassManagerBuilder. Not nec

[PATCH] D27157: IRGen: Remove all uses of CreateDefaultAlignedLoad.

pcc created this revision. pcc added a reviewer: rsmith. pcc added subscribers: cfe-commits, rjmccall. https://reviews.llvm.org/D27157 Files: clang/lib/CodeGen/CGBuilder.h clang/lib/CodeGen/CGBuiltin.cpp clang/lib/CodeGen/CGCall.cpp clang/lib/CodeGen/CGObjCGNU.cpp clang/lib/CodeGen/Targ

[PATCH] D27157: IRGen: Remove all uses of CreateDefaultAlignedLoad.

pcc updated this revision to Diff 79436. pcc marked an inline comment as done. pcc added a comment. - Address review comments https://reviews.llvm.org/D27157 Files: clang/lib/CodeGen/CGBuilder.h clang/lib/CodeGen/CGBuiltin.cpp clang/lib/CodeGen/CGCall.cpp clang/lib/CodeGen/CGObjCGNU.cpp

[PATCH] D27157: IRGen: Remove all uses of CreateDefaultAlignedLoad.

pcc added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2195 LoadInst *Load = -Builder.CreateDefaultAlignedLoad(IntToPtr, /*isVolatile=*/true); +Builder.CreateAlignedLoad(IntTy, IntToPtr, CharUnits::fromQuantity(4)); +Load->setVolatile(true

[PATCH] D27157: IRGen: Remove all uses of CreateDefaultAlignedLoad.

pcc added inline comments. Comment at: clang/lib/CodeGen/CGBuilder.h:126 // FIXME: these "default-aligned" APIs should be removed, // but I don't feel like fixing all the builtin code right now. llvm::StoreInst *CreateDefaultAlignedStore(llvm::Value *Val, ---

[PATCH] D27157: IRGen: Remove all uses of CreateDefaultAlignedLoad.

This revision was automatically updated to reflect the committed changes. Closed by commit rL288083: IRGen: Remove all uses of CreateDefaultAlignedLoad. (authored by pcc). Changed prior to commit: https://reviews.llvm.org/D27157?vs=79436&id=79462#toc Repository: rL LLVM https://reviews.llvm

[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.

pcc updated this revision to Diff 80709. pcc added a comment. Refresh https://reviews.llvm.org/D22296 Files: clang/include/clang/AST/VTableBuilder.h clang/lib/AST/VTableBuilder.cpp clang/lib/CodeGen/CGCXX.cpp clang/lib/CodeGen/CGVTT.cpp clang/lib/CodeGen/CGVTables.cpp clang/lib/Code

[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.

pcc added a reviewer: rjmccall. pcc added a comment. John, may I ask you to take a look? https://reviews.llvm.org/D22296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24431: CodeGen: Start using inrange annotations on vtable getelementptr.

pcc added a reviewer: rjmccall. pcc added a comment. John, may I ask you to take a look? https://reviews.llvm.org/D24431 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.

pcc added inline comments. Comment at: clang/include/clang/AST/VTableBuilder.h:225 private: + std::vector VTableIndices; + rjmccall wrote: > Does this actually grow, or should you use a representation more like > VTableComponents? Or something custom that exp

[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.

pcc updated this revision to Diff 80844. pcc marked 6 inline comments as done. pcc added a comment. - Address review comments https://reviews.llvm.org/D22296 Files: clang/include/clang/AST/VTableBuilder.h clang/lib/AST/VTableBuilder.cpp clang/lib/CodeGen/CGCXX.cpp clang/lib/CodeGen/CGVT

[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.

pcc updated this revision to Diff 81264. pcc added a comment. - Address review comments https://reviews.llvm.org/D22296 Files: clang/include/clang/AST/VTableBuilder.h clang/include/clang/Basic/LLVM.h clang/lib/AST/VTableBuilder.cpp clang/lib/CodeGen/CGCXX.cpp clang/lib/CodeGen/CGVTT.c

[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.

pcc marked 3 inline comments as done. pcc added inline comments. Comment at: clang/include/clang/AST/VTableBuilder.h:255 +operator ArrayRef() const { return {data(), size()}; }; + }; + rjmccall wrote: > Maybe this ought to be in LLVM as OwnedArrayRef? And t

[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.

This revision was automatically updated to reflect the committed changes. pcc marked an inline comment as done. Closed by commit rL289584: CodeGen: New vtable group representation: struct of vtable arrays. (authored by pcc). Changed prior to commit: https://reviews.llvm.org/D22296?vs=81264&id=8

[PATCH] D24431: CodeGen: Start using inrange annotations on vtable getelementptr.

This revision was automatically updated to reflect the committed changes. Closed by commit rL289585: CodeGen: Start using inrange annotations on vtable getelementptr. (authored by pcc). Changed prior to commit: https://reviews.llvm.org/D24431?vs=75494&id=81289#toc Repository: rL LLVM https:

[PATCH] D32243: Fix a leak in tools/driver/cc1as_main.cpp

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

[PATCH] D32064: [asan] Disable ASan global-GC depending on the target and compiler flags

pcc added a comment. I won't stand in the way here if others feel strongly that the flag should be passed via a constructor. It will just mean more work to be done if/when this flag is ever changed to be passed via some other mechanism, but that's a relatively minor detail. Repository: rL L

[PATCH] D32064: [asan] Disable ASan global-GC depending on the target and compiler flags

pcc added a comment. Please add a test case. Repository: rL LLVM https://reviews.llvm.org/D32064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32064: [asan] Disable ASan global-GC depending on the target and compiler flags

pcc added a comment. I think the only functional change here is for COFF, so you can add a CodeGen test that checks that metadata globals are created only if `-fdata-sections` is passed. Repository: rL LLVM https://reviews.llvm.org/D32064 ___ c

[PATCH] D32064: [asan] Disable ASan global-GC depending on the target and compiler flags

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

[PATCH] D32514: [asan] Unconditionally enable GC of globals on COFF

pcc added a comment. Why? Repository: rL LLVM https://reviews.llvm.org/D32514 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32886: [asan] A clang flag to enable ELF globals-gc

pcc accepted this revision. pcc added a comment. LGTM Unfortunate, but I guess this is in the same category as things like `-fsized-deallocation`. Comment at: lib/Driver/SanitizerArgs.cpp:573 + +// This is a workaround for a bug in gold and enabled by default for non-ELF

[PATCH] D33134: Remove ignore-empty-index-file option

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

[PATCH] D118171: [HWASan] Add __hwasan_init to .preinit_array.

pcc added a comment. What guarantee do we have that the `__hwasan_init` .preinit_array entry will be placed before the user's .preinit_array entries? Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:842 + if (!Args.hasArg(options::OPT_shared)) +SharedRuntime

[PATCH] D120862: Sema: Allow scoped enums as source type for integral conversion.

pcc created this revision. pcc added a reviewer: rsmith. Herald added a project: All. pcc requested review of this revision. Herald added a project: clang. Fixes pr54158. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D120862 Files: clang/lib/Sema/SemaOverload.cpp clang/tes

[PATCH] D116773: AST: Make getEffectiveDeclContext() a member function of ItaniumMangleContextImpl. NFCI.

pcc created this revision. pcc added reviewers: eugenis, rsmith, jrtc27. pcc requested review of this revision. Herald added a project: clang. In an upcoming change we are going to need to access mangler state from the getEffectiveDeclContext() function. Therefore, make it a member function of Ita

[PATCH] D116774: AST: Move __va_list tag to the top level on ARM architectures.

pcc created this revision. pcc added reviewers: rsmith, eugenis, jrtc27. Herald added a subscriber: kristof.beyls. pcc requested review of this revision. Herald added a project: clang. In post-commit feedback on D104830 Jessica Clarke pointed out that unconditio

[PATCH] D104830: AST: Create __va_list in the std namespace even in C.

pcc added a comment. In D104830#3225370 , @jrtc27 wrote: > Ping? Sorry for the delay, D116774 should fix this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104830/new/

[PATCH] D116774: AST: Move __va_list tag to the top level on ARM architectures.

pcc added a comment. I'm not aware of any of those places causing an actual problem though? The AST isn't a stable interface, and __builtin_dump_struct is for debugging purposes only. I would expect debug info consumers to be able to handle __va_list in the global namespace as this is the statu

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

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

[PATCH] D92892: [clang] Change builtin object size to be compatible with GCC when sub-object is invalid

pcc added a comment. This causes us to reject the following (reduced from AOSP): int sprintf(char* __s, const char* __fmt, ...) __attribute__((__format__(printf, 2, 3))) ; int sprintf(char* dest, const char* format) __attribute__((overloadable)) __attribute__((enable_if(((__

[PATCH] D42225: libcxx: Provide overloads for basic_filebuf::open() et al that take wchar_t* filenames on Windows.

pcc added subscribers: thomasanderson, thakis. pcc added a comment. I believe that Chromium uses it (or at least it did at the time that I added this, and Chromium has since switched to using libc++ on Windows). I don't work on Chromium much anymore but perhaps @thakis or @thomasanderson can co

<    3   4   5   6   7   8