[PATCH] D50099: [DebugInfo][OpenCL] Address post-commit review of D49930

echristo added a comment. In https://reviews.llvm.org/D50099#1189667, @scott.linder wrote: > When I went to mark these as static I noticed they use > `CGDebugInfo::CreateMemberType` which uses a couple other non-static member > functions, and it starts to become difficult to tease things out in

[PATCH] D50099: [DebugInfo][OpenCL] Address post-commit review of D49930

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

[PATCH] D51150: [x86/retpoline] Split the LLVM concept of retpolines into separate subtarget features for indirect calls and indirect branches.

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

[PATCH] D47070: [CUDA] Upgrade linked bitcode to enable inlining

echristo added a comment. In https://reviews.llvm.org/D47070#1105533, @Hahnfeld wrote: > Looks like this was added as a "temporary solution" in > https://reviews.llvm.org/D8984. Meanwhile the attribute whitelist was merged > half a year later (https://reviews.llvm.org/D7802), so maybe we can ju

[PATCH] D45783: [DEBUGINFO, NVPTX] Render `-no-cuda-debug` LLVM option when required.

echristo added a comment. So, I'd really prefer not to set options via the backend option path. From here I think we should aim to take all of the options we added and having the asm printer in the backend know how to set them depending on the target. We could also add things to the IR metadata

[PATCH] D47029: [X86] Remove some preprocessor feature checks from intrinsic headers

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

[PATCH] D46791: Make -gsplit-dwarf generally available

echristo added subscribers: pcc, paulsemel. echristo added a comment. FWIW Peter has some patches to move object emission away from objcopy that I'm on the hook to review here shortly so the objcopy part of this should become unnecessary and can just have us able to emit dwarf5 compatible split

[PATCH] D47050: MC: Change the streamer ctors to take an object writer instead of a stream. NFCI.

echristo added a comment. LGTM. Repository: rC Clang https://reviews.llvm.org/D47050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46230: For x86_64, gcc 7.2 under Amazon Linux AMI sets its paths to x86_64-amazon-linux

echristo added a comment. Well, that's ridiculous. We should really fix this a better way in the future. That said, would you add a testcase for this please so we don't regress if we fix it? :) -eric Repository: rC Clang https://reviews.llvm.org/D46230 __

[PATCH] D46541: [CodeGen] Improve diagnostics related to target attributes

echristo added a comment. I think this will work, one inline comment. Might also be good to get a few different test cases, e.g. one where we're not seeing the alphabetically first as the minimum :) Comment at: lib/CodeGen/CodeGenModule.h:1085 + TargetAttr::ParsedTargetAtt

[PATCH] D46791: Make -gsplit-dwarf generally available

echristo added a comment. In https://reviews.llvm.org/D46791#1107168, @pcc wrote: > There were a bunch of them but the last one was > https://reviews.llvm.org/D47093 which has already landed :) > > Probably all that needs to happen on this change is to replace the isLinux() > check with isELF()

[PATCH] D38596: Implement attribute target multiversioning

echristo added a comment. This generally works for me modulo the things that Hal has mentioned. You'll probably want to add Richard to the review list for the Sema bits as well. Thanks! Comment at: lib/Sema/SemaDecl.cpp:3214 if (!isFriend && NewMethod->getLexicalDeclCo

[PATCH] D38903: [ubsan] Only use indirect RTTI in prologues on Darwin

echristo added a comment. Given you were the last one in this code it seems reasonable to let you go for it :) That said, I didn't notice anything in particular that stuck out at me. https://reviews.llvm.org/D38903 ___ cfe-commits mailing list cfe

[PATCH] D39179: [AArch64] Fix PR34625 -mtune without -mcpu should not set -target-cpu

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. This should be fine for now. Thanks! -eric https://reviews.llvm.org/D39179 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

[PATCH] D51177: [DEBUGINFO] Add support for emission of the debug directives only.

echristo added a comment. Should we just have them mean the same thing and change it based on target? Repository: rC Clang https://reviews.llvm.org/D51177 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D51007: Test the cross-product of options that affect how libgcc-related arguments are passed to the linker.

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Feel free to add any tests like this that test existing behavior without review in the future. If we want to change the behavior we should probably review that though :) -eric Repositor

[PATCH] D51521: Refactor Addlibgcc to make the when and what logic more straightfoward.

echristo added a comment. LGTM, but let's get Stephen to ack as well. Repository: rC Clang https://reviews.llvm.org/D51521 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51554: [CUDA][OPENMP][NVPTX]Improve logic of the debug info support.

echristo added a reviewer: dblaikie. echristo added a comment. The change in name here from "line tables" to "directives only" feels a bit confusing. "Limited" seems to be a bit more clear, or even remaining line tables only. Can you explain where you were going with this particular set of cha

[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.

echristo added a comment. Getting close, one inline comment. Comment at: lib/Driver/ToolChains/Cuda.h:161 bool SupportsProfiling() const override { return false; } + bool supportsDebugInfoOption(const llvm::opt::Arg *) const override { +return false; I'

[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.

echristo added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:933-938 + if (TC.supportsDebugInfoOption(A)) { +Action(); +return true; + } + reportUnsupportedDebugInfoOption(A, Args, D, TC.getTriple()); + return false; I'd probably simpli

[PATCH] D49828: [libc++] Add hack to allow ubsan to work w/o compiler-rt (__muloti4 is undefined)

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Bit of a hack, but I'm ok with it. Repository: rCXX libc++ https://reviews.llvm.org/D49828 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D49930: [DebugInfo][OpenCL] Generate correct block literal debug info for OpenCL

echristo added a comment. The patch is fine, in general, couple of comments: a) Can you refactor this so the if conditionals are just two functions? Those functions are big enough already. b) I'm not quite sure why you're picking limited here, do you have an explanation? c) Can you split that p

[PATCH] D49930: [DebugInfo][OpenCL] Generate correct block literal debug info for OpenCL

echristo added a comment. In https://reviews.llvm.org/D49930#1181000, @scott.linder wrote: > Sorry, I didn't see the additional comments until after I committed. I will > make those changes; is it OK to update this review, or should I create a new > one? A new one is great. Just treat this as

[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.

echristo added a comment. In https://reviews.llvm.org/D49148#1184957, @tra wrote: > I wonder, what's the right thing to do to silence the warnings. For instance, > we compile everything with -Werror and the warnings result in build breaks. > > Easy way out is to pass `-Wno-unsupported-target-opt

[PATCH] D50099: [DebugInfo][OpenCL] Address post-commit review of D49930

echristo added a comment. Looks pretty good. Could you pass CGM in and just make the functions static I couldn't see any other class variables, but might have missed something. One inline comment as well. -eric Comment at: lib/CodeGen/CGDebugInfo.cpp:997 llvm::DINode::DIF

[PATCH] D43045: Add NVPTX Support to ValidCPUList (enabling march notes)

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. LGTM with an inline comment. Comment at: include/clang/Basic/Cuda.h:49 SM_72, + LAST, }; We have last, invalid, etc... maybe we should pick one amon

[PATCH] D43057: Add Rest of Targets Support to ValidCPUList (enabling march notes)

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. I've glanced at this and didn't notice anything. I think any problems can be fixed in post-commit review if necessary. https://reviews.llvm.org/D43057 _

[PATCH] D43095: Make attribute-target on a Definition-after-use update the LLVM attributes

echristo added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:1318-1325 +llvm::AttrBuilder Attrs; +if (GetCPUAndFeaturesAttributes(D, Attrs)) { + // We know that GetCPUAndFeaturesAttributes will always have the + // newest set, since

[PATCH] D43095: Make attribute-target on a Definition-after-use update the LLVM attributes

echristo added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:1318-1325 +llvm::AttrBuilder Attrs; +if (GetCPUAndFeaturesAttributes(D, Attrs)) { + // We know that GetCPUAndFeaturesAttributes will always have the + // newest set, since

[PATCH] D43359: Clean up 'target' attribute diagnostics

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Seems ok here. https://reviews.llvm.org/D43359 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D43362: Simplify setting dso_local. NFC.

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. LGTM. Thanks for the cleanup. https://reviews.llvm.org/D43362 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D43514: Start settinng dso_local for COFF

echristo added a comment. Some inline comments - I think this looks ok in general, but I'm curious about the answers to the questions/documentation bits inline. Thanks! Comment at: lib/CodeGen/CGDecl.cpp:257 + setGVProperties(GV, &D); + If positioning is i

[PATCH] D43514: Start settinng dso_local for COFF

echristo added inline comments. Comment at: lib/CodeGen/CodeGenModule.h:728 + /// This must be called after dllimport/dllexport is set. + /// FIXME: should this set dllimport/dllexport instead? void setGVProperties(llvm::GlobalValue *GV, const NamedDecl *D) const; --

[PATCH] D33448: [CodeGen] Add thumb-mode to function target-features for arm/thumb triples.

echristo added a comment. In https://reviews.llvm.org/D33448#763411, @fhahn wrote: > In https://reviews.llvm.org/D33448#762410, @echristo wrote: > > > I probably would have added this as a feature in ARMTargetInfo similar to > > CRC/soft-float/etc. > > > > Thoughts? > > > Do you mean ARMTargetMa

[PATCH] D33448: [CodeGen] Add thumb-mode to target-features for arm/thumb triples.

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. One minor nit and LGTM. Thanks! -eric Comment at: lib/Basic/Targets.cpp:5353 +// enable or disable thumb-mode per function +if (isThumb()) Mi

[PATCH] D33448: [CodeGen] Add thumb-mode to target-features for arm/thumb triples.

echristo added a comment. In https://reviews.llvm.org/D33448#765749, @fhahn wrote: > I'll hold off merging this patch until https://reviews.llvm.org/D33436 lands, > which fixes a problem with mixed ARM/Thumb codegen OK. Commit at will :) -eric https://reviews.llvm.org/D33448

[PATCH] D33721: [ARM] Add support for target("arm") and target("thumb").

echristo added inline comments. Comment at: include/clang/Basic/Attr.td:1790-1794 +// target features respectively. +if (Feature.compare("arm") == 0) + Ret.first.push_back("-thumb-mode"); +else if (Feature.compare("thumb") == 0) + Ret.fir

[PATCH] D33721: [ARM] Add support for target("arm") and target("thumb").

echristo added inline comments. Comment at: lib/Basic/Targets.cpp:5439-5442 +// [-|+]thumb-mode target features respectively. +std::vector UpdatedFeaturesVec(FeaturesVec); +for (auto &Feature : UpdatedFeaturesVec) { + if (Feature.compare("+arm") == 0) ---

[PATCH] D33820: [PowerPC] Pass CPU to assembler with -no-integrated-as

echristo added a comment. One inline comment otherwise LGTM Comment at: lib/Driver/ToolChains/Gnu.cpp:681 CmdArgs.push_back("-mppc"); -CmdArgs.push_back("-many"); +std::string CPU = getCPUName(Args, getToolChain().getTriple()); +CmdArgs.push_back(ppc::getPPCAsm

[PATCH] D33721: [ARM] Add support for target("arm") and target("thumb").

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Ah right. Thanks for looking. LGTM. -eric https://reviews.llvm.org/D33721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

[PATCH] D34022: Repair 2010-05-31-palignr.c test

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Thanks for the fix! Do you need someone to commit it? https://reviews.llvm.org/D34022 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D34022: Repair 2010-05-31-palignr.c test

echristo added a comment. Thanks! https://reviews.llvm.org/D34022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34304: Allow CompilerInvocations to generate .d files.

echristo edited reviewers, added: bkramer; removed: echristo. echristo added a comment. Going to let Ben review this. I'd rather not pass the bool down and he might know a way to avoid that here by knowing the code more :) https://reviews.llvm.org/D34304 _

[PATCH] D34425: Unified ARM logic for computing target ABI.

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. This is OK once the dependent revision is approved. https://reviews.llvm.org/D34425 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Couple of inline comments, otherwise I'm pretty happy. I'd wait for an ack by Richard for this though. -eric Comment at: lib/CodeGen/CGBuiltin.cpp:7673 -Value *CodeGe

[PATCH] D41837: Add Function multiversion to the release notes.

echristo added a comment. I think you're missing that right now it's x86 only yes? :) -eric Repository: rC Clang https://reviews.llvm.org/D41837 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

[PATCH] D43851: Start setting dllimport/dllexport in setGVProperties

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Seems reasonable to me. https://reviews.llvm.org/D43851 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

echristo added inline comments. Comment at: test/Sema/attr-print.cpp:3 +// This file is also used as input for %S/../Frontend/ast-attr.cpp. + Relatedly I don't think we use files as input files to other directories and I don't think we should really. What are

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

echristo added inline comments. Comment at: test/Sema/attr-print.cpp:3 +// This file is also used as input for %S/../Frontend/ast-attr.cpp. + jdenny wrote: > echristo wrote: > > Relatedly I don't think we use files as input files to other directories > > and I

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

echristo added a comment. In https://reviews.llvm.org/D43248#1036426, @aaron.ballman wrote: > In https://reviews.llvm.org/D43248#1036409, @jdenny wrote: > > > I'd prefer to move it than to expect people to obey such a comment. Let's > > see what Aaron says. > > > I have a slight preference for

[PATCH] D52920: Introduce code_model macros

echristo added a comment. LGTM as well. Thanks! Repository: rL LLVM https://reviews.llvm.org/D52920 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53248: [Driver] Support direct split DWARF emission for Fuchsia

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. I'm ok with the "here are a bunch of fuchsia tests" file :) -eric Repository: rC Clang https://reviews.llvm.org/D53248 ___ cfe-commits ma

[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

echristo added a reviewer: aprantl. echristo added a comment. I think Adrian has looked at this more recently than I have. Adding him here. Repository: rC Clang https://reviews.llvm.org/D38061 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D52441: [CodeGen] Update min-legal-vector width based on function argument and return types

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D52441#1271545, @rnk wrote: > In https://reviews.llvm.org/D52441#1258317, @craig.topper wrote: > > > Address Reid's comments. Add a comment with a list of all th

[PATCH] D53586: Implement Function Multiversioning for Non-ELF Systems.

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. All of the target specific stuff looks fine to me. I'm going to defer to rnk about the windows side of things and aaron for the attributes. https://reviews.llvm.org/D53586

[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.

echristo added a comment. In https://reviews.llvm.org/D52296#1246142, @grimar wrote: > In https://reviews.llvm.org/D52296#1243688, @echristo wrote: > > > In https://reviews.llvm.org/D52296#1241928, @probinson wrote: > > > > > Do we generate the .dwo file directly these days? If not, I can imagin

[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value on x86-64.

echristo added a comment. In https://reviews.llvm.org/D53919#1282994, @hjl.tools wrote: > In https://reviews.llvm.org/D53919#1282952, @efriedma wrote: > > > With both 3.3 and trunk (I don't have a 7.0 handy; I can build it if it > > would be helpful): > > > Please try clang 2.6 on both testcases

[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.

echristo added a comment. In https://reviews.llvm.org/D52296#1285328, @grimar wrote: > In https://reviews.llvm.org/D52296#1284130, @probinson wrote: > > > In https://reviews.llvm.org/D52296#1283691, @grimar wrote: > > > > > Nice :) > > > So seems the last unresolved question left is the naming

[PATCH] D54243: DebugInfo: Add a driver flag for DWARF debug_ranges base address specifier use.

echristo added a comment. I have no other bikeshed colors here. :) Repository: rC Clang https://reviews.llvm.org/D54243 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51554: [CUDA][OPENMP][NVPTX]Improve logic of the debug info support.

echristo added a comment. The llvm backend patch here has discussion around debug info kinds that we should iron out first. Comment at: lib/Driver/ToolChains/Cuda.cpp:292 + bool IsDebugEnabled = !A || A->getOption().matches(options::OPT_O0) || +Args.ha

[PATCH] D55016: Correctly support -shared-libgcc.

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. LGTM. -eric Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55016/new/ https://reviews.llvm.org/D55016 ___ cf

[PATCH] D51554: [CUDA][OPENMP][NVPTX]Improve logic of the debug info support.

echristo added inline comments. Comment at: lib/Driver/ToolChains/Cuda.cpp:282-285 enum DebugInfoKind { - NoDebug, /// No debug info. - LineTableOnly, /// Line tables only. - FullDebug /// Full debug info. + NoDebug, /// No debug info. + DebugDirectiv

[PATCH] D61464: [RiscV] Typo in register aliases

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. LGTM. Sorry I didn't notice this earlier. -eric CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61464/new/ https://reviews.llvm.org/D61464 __

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

echristo added reviewers: saugustine, cmatthews. echristo added a comment. Adding Sterling and Chris to this to take a look at the new layout :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59168/new/ https://reviews.llvm.org/D59168 ___ cf

[PATCH] D61464: [RiscV] Typo in register aliases

This revision was automatically updated to reflect the committed changes. Closed by commit rL360104: Fix typo in risc-v register aliases. (authored by echristo, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.o

[PATCH] D52280: Don't trim non-alphanumeric characters in 'file not found' errors for include directives.

This revision was automatically updated to reflect the committed changes. Closed by commit rC342667: r342177 introduced a hint in cases where an #included file is not found. It… (authored by echristo, committed by ). Changed prior to commit: https://reviews.llvm.org/D52280?vs=166184&id=166327#t

[PATCH] D52280: Don't trim non-alphanumeric characters in 'file not found' errors for include directives.

This revision was automatically updated to reflect the committed changes. Closed by commit rL342667: r342177 introduced a hint in cases where an #included file is not found. It… (authored by echristo, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://revi

[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.

echristo added a comment. In https://reviews.llvm.org/D52296#1241928, @probinson wrote: > Do we generate the .dwo file directly these days? If not, I can imagine > wanting to avoid the overhead of the objcopy hack; as long as the linker is > smart enough not to bother with the .debug_*.dwo sec

[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.

echristo added a comment. In https://reviews.llvm.org/D52296#1241928, @probinson wrote: > Do we generate the .dwo file directly these days? If not, I can imagine > wanting to avoid the overhead of the objcopy hack; as long as the linker is > smart enough not to bother with the .debug_*.dwo sec

[PATCH] D52441: [CodeGen] Update min-legal-vector width based on function argument and return types

echristo added a comment. Couple of comments/questions: a) How do you want these attributes to be handled in merging/inlining? Also, are they a failure on module linking? In general, how do they work with LTO? b) Could use some more comments when we're adding/merging the attributes in IR genera

[PATCH] D36336: [X86] Add support for __builtin_cpu_init

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. One inline comment, but go ahead and commit after fixing that up. Comment at: lib/CodeGen/CGBuiltin.cpp:7292 const CallExpr *E

[PATCH] D36057: Use class to pass information about executable name

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Fine with me. -eric https://reviews.llvm.org/D36057 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/ma

[PATCH] D37428: Debug info: Fixed faulty debug locations for attributed statements

echristo added inline comments. Comment at: test/CodeGen/debug-info-attributed-stmt.c:1 +// RUN: %clang_cc1 -triple x86_64-unk-unk -disable-llvm-passes -debug-info-kind=limited -emit-llvm %s -o - | FileCheck %s + Since we're not optimizing or generating code you

[PATCH] D36431: Add powerpc64 to compiler-rt build infrastructure.

echristo accepted this revision. echristo added a comment. This is great with me. https://reviews.llvm.org/D36431 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37656: [cfi] Set function attributes for __cfi_* functions.

echristo added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:2917 +CodeGenFunction &CGF, llvm::Function *F, +bool ForceThumb) { + StringRef TargetCPU = CGF.getTarget().getTargetOpts().CPU; --

[PATCH] D37656: [cfi] Set function attributes for __cfi_* functions.

echristo added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:2917 +CodeGenFunction &CGF, llvm::Function *F, +bool ForceThumb) { + StringRef TargetCPU = CGF.getTarget().getTargetOpts().CPU; --

[PATCH] D62133: test/CodeGen/builtin-stackaddress.c duplicates test/CodeGen/2004-02-13-BuiltinFrameReturnAddress.c

echristo added a comment. done thusly: echristo@jhereg ~/s/llvm-project> git llvm push Pushing 1 commit: d91813ca8c7 Remove test/CodeGen/builtin-stackaddress.c as it duplicates test/CodeGen/2004-02-13-BuiltinFrameReturnAddress.c. Deleting cfe/trunk/test/CodeGen/builtin-stackaddress.c C

[PATCH] D62133: test/CodeGen/builtin-stackaddress.c duplicates test/CodeGen/2004-02-13-BuiltinFrameReturnAddress.c

This revision was automatically updated to reflect the committed changes. Closed by commit rL362462: Remove test/CodeGen/builtin-stackaddress.c as it duplicates (authored by echristo, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D64540: [CGDebugInfo] Simplfiy EmitFunctionDecl parameters, NFC

echristo added a comment. Not a huge fan of boolean parameters like this, perhaps factor out the context as well into the caller and then we don't need it at all? Something else? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64540/new/ https://reviews.llvm.org/D64540 ___

[PATCH] D65410: [PassManager] First Pass implementation at -O1 pass pipeline

echristo created this revision. echristo added reviewers: chandlerc, hfinkel. Herald added subscribers: cfe-commits, jfb, dexonsmith, steven_wu, hiraditya, javed.absar, mcrosier, mehdi_amini. Herald added projects: clang, LLVM. As a follow-up to my initial mail to llvm-dev here's a first pass at

[PATCH] D65410: [PassManager] First Pass implementation at -O1 pass pipeline

echristo added a comment. In D65410#1613555 , @hfinkel wrote: > Thanks for starting on this. Can you go ahead and replace the sroa calls with > mem2reg calls for `O1` and then see what that does to the performance? That > strikes me as a major change, bu

[PATCH] D62225: [clang][NewPM] Fixing -O0 tests that are broken under new PM

echristo added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1104-1105 // which is just that always inlining occurs. - MPM.addPass(AlwaysInlinerPass()); + // We always pass false here since according to the legacy PM logic for + // enabling

[PATCH] D62133: test/CodeGen/builtin-stackaddress.c duplicates test/CodeGen/2004-02-13-BuiltinFrameReturnAddress.c

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. LGTM. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62133/new/ https://reviews.llvm.org/D62133 ___ cfe-commi

[PATCH] D51554: [CUDA][OPENMP][NVPTX]Improve logic of the debug info support.

echristo accepted this revision. echristo added a comment. LGTM. I'm quite a bit happier with this now. Thanks for going through the back and forth. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51554/new/ https://reviews.llvm.org/D51554 _

[PATCH] D56044: [Driver] Support object files in addition to static and shared libraries in compiler-rt

echristo added a comment. Is something passing compiler-rt in as a -r link output or something? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56044/new/ https://reviews.llvm.org/D56044 ___ cfe-commits mailing list cf

[PATCH] D56044: [Driver] Support object files in addition to static and shared libraries in compiler-rt

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. OK, that makes sense. I'm not a huge fan of how this set of code looks, but it also seems unfair to require you to extensively refactor it for this. Any issues with crtbegin/end and where

[PATCH] D56043: [Driver] Don't pass default value to getCompilerRTArgString

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. LGTM, thanks. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56043/new/ https://reviews.llvm.org/D56043 ___ c

[PATCH] D56181: [CMake][Fuchsia] Include check-lld in the list of bootstrap targets

echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. This seems like the sort of thing that you can just commit in the future. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56181/new/ https://reviews.llvm.org

[PATCH] D67627: Clang-format: Add Whitesmiths indentation style

echristo added reviewers: mboehme, krasimir. echristo added a comment. Martin should know who should look at this... maybe Krasimir? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67627/new/ https://reviews.llvm.org/D67627 ___

[PATCH] D46410: [Target] Diagnose mismatch in required CPU for always_inline functions

echristo added a comment. I like the idea of a front end warning, but had believed that a subset of cpu features should be allowed to be inlined into something that's a superset and it sounds like you don't agree? Your thoughts here? -eric https://reviews.llvm.org/D46410 __

[PATCH] D48100: Append new attributes to the end of an AttributeList.

echristo added subscribers: dlj, echristo. echristo added a comment. I've added a couple of inline comments here - between this and the comments in the post-commit review from dlj it seems like we might want to revert this for now and figure out the best way forward. Thanks! -eric =

[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.

echristo added a comment. I think you should break it out on an option by option basis. Just warning on "non-standard" options doesn't really make sense. Repository: rC Clang https://reviews.llvm.org/D49148 ___ cfe-commits mailing list cfe-commi

[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.

echristo added a comment. In https://reviews.llvm.org/D49148#1166429, @ABataev wrote: > In https://reviews.llvm.org/D49148#1165826, @echristo wrote: > > > I think you should break it out on an option by option basis. Just warning > > on "non-standard" options won't make as much sense to end user

[PATCH] D46230: For x86_64, gcc 7.2 under Amazon Linux AMI sets its paths to x86_64-amazon-linux

echristo added a comment. It's usually preferred to submit patches with full context - it'll make this a bit easier if you do please? Repository: rC Clang https://reviews.llvm.org/D46230 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.

echristo added a comment. In https://reviews.llvm.org/D49148#1166859, @echristo wrote: > In https://reviews.llvm.org/D49148#1166429, @ABataev wrote: > > > In https://reviews.llvm.org/D49148#1165826, @echristo wrote: > > > > > I think you should break it out on an option by option basis. Just > >

[PATCH] D46230: For x86_64, gcc 7.2 under Amazon Linux AMI sets its paths to x86_64-amazon-linux

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

[PATCH] D46230: For x86_64, gcc 7.2 under Amazon Linux AMI sets its paths to x86_64-amazon-linux

echristo added a comment. In https://reviews.llvm.org/D46230#1166958, @gaijiading wrote: > In https://reviews.llvm.org/D46230#1166919, @echristo wrote: > > > LGTM. > > > > -eric > > > Hi Eric, I do not have commit access to trunk. Could you commit the change > for me? Thanks. The binaries make

[PATCH] D68535: Fix loop unrolling initialization in the new pass manager

echristo created this revision. echristo added reviewers: chandlerc, hfinkel. Herald added subscribers: llvm-commits, cfe-commits, hiraditya, mcrosier. Herald added projects: clang, LLVM. In the new pass manager use PTO.LoopUnrolling to determine when and how we will unroll loops. Also comment a f

[PATCH] D68255: [X86] Remove AVX/AVX512 check from validateOperandSize, just always accept 512

echristo added a comment. The idea was that we'd have a much more graceful error message as well as a way for the front end to do the diagnosis. The code is being called out of SemaStmtAsm.cpp and I'm pretty sure we can probably accumulate the target attributes there? -eric CHANGES SINCE LAS

[PATCH] D66795: [Mips] Use appropriate private label prefix based on Mips ABI

echristo added a comment. I kinda want the option to be encoded in the triple for earlier testing of linking issues, but for now this is probably OK. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66795/new/ https://reviews.llvm.org/D66795

  1   2   3   >