[PATCH] D54756: [DebugInfo] NFC Clang test changes for DISubprogram flags

probinson added a comment. In D54756#1311679 , @aprantl wrote: > Everybody with out-of-tree tests will be excited ;-) Yeah... fortunately we still accept the old syntax on input, so it's only people with checks on DISubprogram *output* that will need to

[PATCH] D54756: [DebugInfo] NFC Clang test changes for DISubprogram flags

This revision was automatically updated to reflect the committed changes. Closed by commit rL347807: [DebugInfo] NFC Clang test changes for: IR/Bitcode changes for DISubprogram… (authored by probinson, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://rev

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

probinson added a comment. > When building the FileCheck binary with debug info, this patch makes the > build artifacts ~1kb smaller. Nice! Comment at: lib/IR/DiagnosticInfo.cpp:39 #include "llvm/Support/ScopedPrinter.h" +#include "llvm/Support/raw_ostream.h" #include

[PATCH] D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries

probinson added inline comments. Comment at: lib/IR/DiagnosticInfo.cpp:39 #include "llvm/Support/ScopedPrinter.h" +#include "llvm/Support/raw_ostream.h" #include aprantl wrote: > probinson wrote: > > Do we use a case-sensitive sort of include files? I thought

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

probinson added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:238 + // Apply -fdebug-prefix-map. + PP.RemapFilePaths = true; + PP.remapPath = [this](StringRef Path) { return remapDIPath(Path); }; Unconditionally? CHANGES SINCE LAST ACTION https:

[PATCH] D55137: Honor -fdebug-prefix-map when creating function names for the debug info.

probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. LGTM aside from a grump about the test, which is totally up to you. Comment at: test/CodeGenCXX/debug-prefix-map-lambda.cpp:7 + // CHECK: !DISubprogram(name: "b<(lambd

[PATCH] D86965: Do not emit "-tune-cpu generic" for PS4 platform

probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. Couple of nits and LGTM. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2078 - // Default to "generic" unless -march is present. + // Default to "generic" unless

[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

probinson added a comment. Looks okay (one grammar nit), probably worth waiting for someone else to chime in. Comment at: clang/docs/ClangCommandLineReference.rst:2139 -Enable stack protectors for some functions vulnerable to stack smashing. This uses a loose heuristic whic

[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

probinson added a comment. Whoa--that's the *help* text? Well, if that's the only documentation for options that there is, I guess it has to go there; but it seems pretty excessive for the (ideally concise) output of `clang --help`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D80242: [Clang] implement -fno-eliminate-unused-debug-types

probinson added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:2155 + +Emit debug info for types that are unused but defined by the program. + I think this description goes counter to how other options are described, which basically is abo

[PATCH] D81970: [Clang][Driver] Remove gold linker support for PS4 toolchain

probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. LGTM with one inline comment. Comment at: clang/lib/Driver/ToolChains/PS4CPU.cpp:154 const char *Exec = -#ifdef _WIN32 - Args.MakeArgString(ToolChain.GetProgram

[PATCH] D74324: Tools emit the bug report URL on crash

probinson added a comment. Hi @leonardchan Owen is on UK time so I took a look. I think Owen inadvertently put the new API in a place that is guarded by `#if ENABLE_BACKTRACES` and I'm guessing that your build has that turned off. I've posted D76893 because so

[PATCH] D77393: [X86] Fix implicit sign conversion warnings in X86 headers.

probinson added a comment. It looks like you're not actually interested in the compiled output, but just whether warnings occurred; in that case you'd be better off with `-verify -fsyntax-only` and `// expected-no-diagnostics`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77393/new/

[PATCH] D78024: [FileCheck] - Fix the false positive when -implicit-check-not is used with an unknown -check-prefix.

probinson added a comment. I'm just reading this review for the first time, and my thought was, why is this interacting with implicit-check-not? I think specifying a `--check-prefix=FOO` with no uses of FOO should be diagnosed. I can't say I recall the previous discussion in detail, but that's

[PATCH] D76377: Correctly initialize the DW_AT_comp_dir attribute of Clang module skeleton CUs

probinson added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2499 +DIB.createFile(Mod.getModuleName(), TheCU->getDirectory()), +TheCU->getProducer(), false, StringRef(), 0, Mod.getASTFile(), +llvm::DICompileUnit::FullDebug, Signature); --

[PATCH] D80876: [clang] Default to windows response files when running on windows

probinson added inline comments. Comment at: clang/tools/driver/driver.cpp:391 +if (ClangCLMode || +llvm::Triple(llvm::sys::getProcessTriple()).isOSWindows()) + Tokenizer = &llvm::cl::TokenizeWindowsCommandLine; Testing the triple is a functional

[PATCH] D80242: [Clang] implement -fno-eliminate-unused-debug-types

probinson added a comment. In D80242#2045362 , @nickdesaulniers wrote: > For C++, I'd imagine: > > class foo {}; > using my_foo = foo; > template > struct baz { > bar my_bar; > }; > > > but it seems that `g++` doesn't emit debug info the fo

[PATCH] D80876: [clang] Default to windows response files when running on windows

probinson added a comment. As a functional change it should definitely have a functional test. Let me double-check on how our stuff behaves, I may be mis-remembering. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80876/new/ https://reviews.llvm.org

[PATCH] D80876: [clang] Default to windows response files when running on windows

probinson added a comment. Indeed I am mis-remembering, sorry about that! We use gnu-style command-line options but we change RSPQuoting from Default to Windows; assuming getProcessTriple() is the host not the target, your change should have the same effect. I'll try removing our private patc

[PATCH] D80876: [clang] Default to windows response files when running on windows

probinson added a subscriber: rnk. probinson added a comment. I was about to add @rnk who I remember has been in Windows driver behavior discussions in the past; except he has cleverly left a note that he's away June-Sept. Oh well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D80876: [clang] Default to windows response files when running on windows

probinson added a comment. It looks like your patch will allow us to remove a private patch that has a similar effect. Incidentally if I apply this to an upstream checkout on Windows, I see clang/test/Driver/at_file.c fails, so you'd at least need to do something for that. (`UNSUPPORTED: system

[PATCH] D80876: [clang] Default to windows response files when running on windows

probinson added a comment. Re testing, you could copy clang/test/Driver/at_file_win.c, which has an explicit `--rsp-quoting=windows`; remove that option and make it `REQUIRES: system-windows`. That should do it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D80242: [Clang] implement -fno-eliminate-unused-debug-types

probinson added a subscriber: debug-info. probinson added a comment. +debug-info tag Needs a Driver test to show that the Right Thing gets passed to cc1. Comment at: clang/docs/CommandGuide/clang.rst:438 + + By default, Clang does not emit type information for types that are

[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

probinson added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1618 + return; +} else if (const auto Def = Method->getDefinition()) { + if (Def->isDefaulted()) { LLVM style is not to use 'else' after 'return'. Comm

[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

probinson added inline comments. Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:91 HANDLE_DISP_FLAG((1u << 8), MainSubprogram) +HANDLE_DISP_FLAG((1u << 9), NotDefaulted) +HANDLE_DISP_FLAG((1u << 10), DefaultedInClass) probinson wrote: > SouraVX wrote: > > a

[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

probinson added a comment. We really do want to pack the four mutually exclusive cases into two bits. I have tried to give more explicit comments inline to explain how you would do this. It really should work fine, recognizing that the "not defaulted" case is not explicitly represented in the

[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

probinson added a comment. In D68117#1709557 , @SouraVX wrote: > Hi David, > I did some digging about DW_AT_defaulted and stuff, not much of a success > but. Here's what I found -- http://dwarfstd.org/Issues.php?type=closed4 -- > here it;s still marke

[PATCH] D67723: [DebugInfo] Add option to disable inline line tables.

probinson added a comment. Apologies for missing this until now. Our email system keeps dropping stuff sent by Phabricator. FTR, since @rnk has mentioned my years-ago writings, what Sony has internally nowadays is a little different than what I said back then. We have an option spelled `-gno

[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

probinson added a comment. In D68117#1710295 , @dblaikie wrote: > I think the existing feature's perhaps a bit misspecified - because where you > put the DEFAULTED_{in_class,out_of_class} would communicate that without > needing the two values - if you p

[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

probinson added a comment. In D68117#1711714 , @dblaikie wrote: > In D68117#1711154 , @probinson wrote: > > > (@dblaikie Aside regarding noreturn, the original DWARF proposal was for a > > debugger wanting to know

[PATCH] D73261: [dwarf5] Support DebugInfo for constexpr for C++ variables and functions

probinson added a comment. I put in a lot of comments about spelling for the new parameter (constExpr, isConstexpr, isConstExpr) which should be named consistently throughout. Please do not use Constant or any variant, as that tends to mean something else. But, what I would rather see instead

[PATCH] D73261: [dwarf5] Support DebugInfo for constexpr for C++ variables and functions

probinson added a comment. In D73261#1838552 , @djtodoro wrote: > > Is it necessary to use DIFlags? I am willing to do that but generally, it > > is not welcomed because we have a limited number of DIFlags and most of > > them are currently in use. > > A

[PATCH] D58033: Add option for emitting dbg info for call site parameters

probinson added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:755 + (Arch == llvm::Triple::x86 || Arch == llvm::Triple::x86_64)) +Opts.EnableDebugEntryValues = Args.hasArg(OPT_femit_debug_entry_values); + You want to disable entry-valu

[PATCH] D63894: [CXX] Exercise all paths through these tests

probinson created this revision. probinson added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. These 3 tests have dialect-based conditionals, but weren't running Clang with enough different dialects to actually enable those conditional sections. Repo

[PATCH] D59347: [DebugInfo] Combine Trivial and NonTrivial flags

probinson added a comment. I'd preserve the trivial test cases and show that the NonTrivial flag is *not* present. I can suggest ways to achieve this if you like (note there is no CHECK-DAG-NOT combined directive). Comment at: test/CodeGenCXX/debug-info-composite-triviality.

[PATCH] D59815: [Driver] Enable -fsanitize-address-globals-dead-stripping by default on PS4.

probinson added a comment. This is fine for PS4, I'm just curious whether anyone knows if there's a predicate that means something like "target does not use GNU tools" so we don't have to itemize Fuschia and PS4 this way. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D59008: [AMDGPU] Switch default dwarf version to 5

probinson added a comment. In D59008#1442515 , @dblaikie wrote: > In D59008#1442256 , @t-tye wrote: > > > In D59008#1442014 , @dblaikie > > wrote: > > > > > In D59008#144190

[PATCH] D59347: [DebugInfo] Combine Trivial and NonTrivial flags

probinson added a comment. As a rule I would prefer flags with positive names, as it's slightly easier to read `!isTrivial` than `!isNonTrivial`. And trivially shorter. :-) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59347/new/ https://reviews.llvm.org/D59347

[PATCH] D59347: [DebugInfo] Combine Trivial and NonTrivial flags

probinson added a comment. In D59347#1444819 , @dblaikie wrote: > In D59347#1443051 , @dblaikie wrote: > > > @asmith: Where's the LLVM-side change/review that goes with this, btw? > > > > In D59347#1442970

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

probinson added a comment. Commentary questions. Comment at: lib/Driver/ToolChains/Clang.cpp:3166 + // If you say "-gsplit-dwarf -gline-tables-only", -gsplit-dwarf loses. + // But -gsplit-dwarf is not a g_Group option, hence we have to check the + // order explic

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. LGTM. I wish it had occurred to me to pass both OPT_g_Group and OPT_gsplit_dwarf to the same getLastArgs call in the first place. Repository: rC Clang CHANGES SINCE LAST ACTION ht

[PATCH] D59815: [Driver] Enable -fsanitize-address-globals-dead-stripping by default on PS4.

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

[PATCH] D59347: [DebugInfo] Combine Trivial and NonTrivial flags

probinson added inline comments. Comment at: test/CodeGenCXX/debug-info-composite-triviality.cpp:88 - -// CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivialE",{{.*}}flags: {{.*}}DIFlagNonTrivial -struct NonTrivialE : Trivial, NonTrivial { Hui wrote: > probins

[PATCH] D58033: Add option for emitting dbg info for call sites

probinson added a comment. I guess I'm not clear what your final goal is for the option. Keep it, even though GCC doesn't have one like it? Eliminate it? Please clearly state what you intend to have in the end, and what you might have in the short term in case that is different. =

[PATCH] D58033: Add option for emitting dbg info for call site parameters

probinson added a comment. In D58033#1470260 , @djtodoro wrote: > @probinson @aprantl Thanks a lot for your comments! > > Let's clarify some things. I'm sorry about the confusion. > > Initial patch for the functionality can be restricted by this option (li

[PATCH] D57162: [DEBUG_INFO][NVPTX] Generate correct data about variable address class.

probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. Herald added a project: clang. LGTM. I'll trust you on the actual address-class values. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57162/new/ https://

[PATCH] D57896: Variable names rule

probinson added a comment. Herald added a project: LLVM. In D57896#1389067 , @lebedev.ri wrote: > 2. It might be best to give this more visibility, by submitting a mail to > llvm-dev, with a **noticeable** subject, like "RFC: changing variable naming > r

[PATCH] D58061: Fix a few tests that were missing ':' on CHECK lines and weren't testing anything.

probinson added a comment. There has been some progress recently on better FileCheck diagnosis of likely test-writing issues, although to date it mostly requires the human to ask "what is going on here?" explicitly. I can see adding a check to detect the missing-colon-on-otherwise-valid-direct

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

probinson added inline comments. Comment at: include/clang/AST/Decl.h:1482 + bool getIsArgumentModified() const { +return IsArgumentModified; There should be a comment here. The style in this class appears to omit the 'get' word from the name of the gette

[PATCH] D57896: Variable names rule

probinson added a comment. In D57896#1406353 , @MyDeveloperDay wrote: > I can't argue with anything you say.. but I guess to reinforce your point > introducing what is effectively a 3rd style would likely cause even more > jarring... Zach isn't introd

[PATCH] D57896: Variable names rule

probinson added a comment. In D57896#1406412 , @MyDeveloperDay wrote: > In D57896#1406407 , @zturner wrote: > > > If I read the post correctly, it was actually agreeing with me (because it > > said "to reinforce y

[PATCH] D60283: [DebugInfo] Don't emit checksums when compiling a preprocessed CPP

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

[PATCH] D62202: Work around a Visual C++ bug

probinson created this revision. probinson added reviewers: aaron.ballman, rnk. Herald added a project: clang. Herald added a subscriber: cfe-commits. Put up for review because I'm not clear whether this should be considered a permanent fix, or if I should put some sort of comment here indicating

[PATCH] D62202: Work around a Visual C++ bug

probinson added a comment. To provide some missing details: The original source gets a bogus compile-time error with Visual Studio 2017, prior to version 15.8. I have 15.2, so I need this patch to build Clang. Our current minimum-version requirement (per CheckCompilerVersion.cmake) is Visual St

[PATCH] D62202: Work around a Visual C++ bug

probinson added a comment. In D62202#1510414 , @dblaikie wrote: > Technically this violates the LLVM style guide which says "make anonymous > namespaces as small as possible, and only use them for class declarations." > (preferring static for functions)

[PATCH] D62202: Work around a Visual C++ bug

probinson added a comment. The helper is passed as a template parameter to a class ctor, and that instantiated class calls the helper from operator(), so I suppose maybe enough indirection through lambdas but this seems kind of involved for getting my builds to work. Repository: rC Clang

[PATCH] D62202: Work around a Visual C++ bug

probinson added a comment. // FIXME: Change the following functions from anonymous namespace to static // after the minimum _MSC_VER >= 1915 (equivalent to Visual Studio version // of 15.8 or higher). Works around a bug in earlier versions. ? Also happy to include the hyperlink to the bug

[PATCH] D62202: Work around a Visual C++ bug

This revision was automatically updated to reflect the committed changes. Closed by commit rL361502: Work around a Visual C++ bug. (authored by probinson, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D62

[PATCH] D62850: [X86] Fix builtins-x86.c test where it wasn't using immediates. NFC

probinson added a comment. Is the compiler missing a check that these parameters are immediates? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62850/new/ https://reviews.llvm.org/D62850 ___ cfe-commits

[PATCH] D58033: Add option for emitting dbg info for call site parameters

probinson added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:3402 +CmdArgs.push_back("-femit-param-entry-values"); + RenderDebugInfoCompressionArgs(Args, CmdArgs, D, TC); If this is now a cc1-only option, this part goes away right? CHANGE

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

probinson added a comment. I'm okay with the PS4-specific bits. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60274/new/ https://reviews.llvm.org/D60274 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/

[PATCH] D58033: Add option for emitting dbg info for call site parameters

probinson added a comment. I'm okay with this, but give @aprantl a chance to confirm. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58033/new/ https://reviews.llvm.org/D58033 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://l

[PATCH] D60283: [DebugInfo] Don't emit checksums when compiling a preprocessed CPP

probinson added a comment. I had tried to do this in r11 but some bots complained, so I reverted it and then didn't follow through. Thanks for doing this! When I tried it, I took advantage of existing tracking of line directives at the file level, so there shouldn't be a need to add a Line

[PATCH] D60283: [DebugInfo] Don't emit checksums when compiling a preprocessed CPP

probinson added a comment. In D60283#1480546 , @aganea wrote: > Thanks Paul, your solution is even better. I'll apply rL11 > locally - if everything's fine, do you > mind if I re-land it again? I suggest you do *

[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

probinson added a comment. FYI, we had to disable clangd entirely after this patch, getting a weird problem with lit that we haven't figured out yet. In case anybody else has seen something like this, and knows what's going on, please let us know. Running all regression tests llvm-lit.p

[PATCH] D61496: Fixed tests where grep was not matching the linefeed

probinson added a comment. Checked-in files should not have CRLF endings, in general (there are a very few exceptions, and these aren't among them). If the checked-in files have LF endings but your tool checks them out with CRLF then that is a problem with your config, or with the tool. Adding d

[PATCH] D60283: [DebugInfo] Don't emit checksums when compiling a preprocessed CPP

probinson added a comment. Minor stuff. This solution is surprisingly simple, the main question being (and I don't have an answer) whether increasing the size of PresumedLoc is okay. Comment at: lib/Basic/SourceManager.cpp:1460 +FID = FileID::get(0); // contents of fi

[PATCH] D15881: [DWARF] Omitting the explicit import of an anonymous namespace is a debugger-tuning decision, not a target decision.

probinson abandoned this revision. probinson added a comment. Herald added a subscriber: JDevlieghere. Abandoning dead patch. This wound up being done a different way. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D15881/new/ https://reviews.llvm.org/D15881 __

[PATCH] D55712: [Driver][PS4] Do not implicitly link against asan or ubsan if -nostdlib or -nodefaultlibs on PS4.

probinson added a comment. LGTM but I will defer to @filcab as he is more familiar with the area. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55712/new/ https://reviews.llvm.org/D55712 ___ cfe-commits mailing list

[PATCH] D55712: [Driver][PS4] Do not implicitly link against asan or ubsan if -nostdlib or -nodefaultlibs on PS4.

probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. I missed that filcab had said ok internally. Go ahead Pierre. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55712/new/ https://reviews.llvm.org/D55712

[PATCH] D56393: [DebugInfo] Don't emit DW_AT_enum_class unless it's actually an 'enum class'.

probinson created this revision. probinson added reviewers: dblaikie, rnk, zturner. probinson added a project: debug-info. Herald added subscribers: cfe-commits, JDevlieghere, aprantl. The original fix for PR36168 would emit DW_AT_enum_class for both of these cases: enum Fixed : short { F1

[PATCH] D56393: [DebugInfo] Don't emit DW_AT_enum_class unless it's actually an 'enum class'.

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rC350636: Don't emit DW_AT_enum_class unless it's actually an 'enum class'. (authored by probinson, committed by ). Changed

[PATCH] D55781: Make CC mangling conditional on the ABI version

probinson added a comment. Herald added a subscriber: mstorsjo. Sorry for not noticing this sooner. The TL;DR is, the current patch does not affect PS4, so go ahead; but see the long version for other considerations. First, the general point: The PS4 ABI does not fit neatly into the sequential

[PATCH] D28620: Guard __gnuc_va_list typedef

This revision was automatically updated to reflect the committed changes. Closed by commit rL292819: Guard __gnuc_va_list typedef. (authored by probinson). Changed prior to commit: https://reviews.llvm.org/D28620?vs=84151&id=85430#toc Repository: rL LLVM https://reviews.llvm.org/D28620 Fil

[PATCH] D24812: Lit C++11 Compatibility Patch #11

probinson added a reviewer: rjmccall. probinson added a comment. +rjmccall as CodeGen owner. https://reviews.llvm.org/D24812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29739: Make Lit tests C++11 compatible - Objective-C++

probinson added a comment. These are all Objective-C++ tests, and AFAIK we don't intend to change the default Objective-C++ dialect when we finally do change the default C++ dialect. So I think these tests do not need to be modified. https://reviews.llvm.org/D29739 __

[PATCH] D29739: Make Lit tests C++11 compatible - Objective-C++

probinson added a comment. In https://reviews.llvm.org/D29739#673971, @rjmccall wrote: > In https://reviews.llvm.org/D29739#673933, @tigerleapgorge wrote: > > > Hi John, > > > > Here is the most recent discussion I can find on cfe-dev. > > “I'm guessing that Objective-C/C++ is kind of passe, so

[PATCH] D29739: Make Lit tests C++11 compatible - Objective-C++

probinson added a comment. In https://reviews.llvm.org/D29739#674297, @rjmccall wrote: > In https://reviews.llvm.org/D29739#674288, @probinson wrote: > > > I really think Apple would need to step up here if the default > > Objective-C++ dialect is going to change. > > > I don't mind stepping up

[PATCH] D27794: Make some diagnostic tests C++11 clean

probinson created this revision. probinson added a reviewer: rsmith. probinson added subscribers: cfe-commits, tigerleapgorge. Another half-dozen test revisions in the ongoing campaign to make things ready for C++11 as Clangs's default dialect. Most of these are straightforward, but I am not ent

[PATCH] D27794: Make some diagnostic tests C++11 clean

probinson added a reviewer: ABataev. probinson added a comment. +abataev for OpenMP. https://reviews.llvm.org/D27794 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27794: Make some diagnostic tests C++11 clean

probinson added a reviewer: rnk. probinson updated this revision to Diff 81977. probinson added a comment. Remove the OpenMP tests from this review (committed in r290128). +rnk who added test/Parser/backtrack-off-by-one.cpp originally. https://reviews.llvm.org/D27794 Files: test/FixIt/fixit.

[PATCH] D27936: C++11 test cleanup: nonthrowing destructors

probinson created this revision. probinson added a reviewer: rsmith. probinson added a subscriber: cfe-commits. If a dtor has no interesting members, then it ends up being nothrow, which affects the generated IR. Modify some tests to tolerate this difference between C++03 and C++11. In C++11, a

[PATCH] D27955: Make CodeGenCXX/arm-swiftcall.cpp tolerate C++11

probinson created this revision. probinson added a reviewer: rjmccall. probinson added a subscriber: cfe-commits. Herald added subscribers: rengolin, aemerson. The test conjures up and returns a temp which has a struct type, and the struct has some empty/padding bytes in the middle. In C++03 the

[PATCH] D27956: Make CodeGenCXX/stack-reuse-miscompile.cpp tolerate C++11

probinson created this revision. probinson added a reviewer: lenykholodov. probinson added a subscriber: cfe-commits. In this test, the allocas for the temps come out in a different order depending on whether the dialect is C++03 or C++11. To avoid depending on the default dialect, I forced it

[PATCH] D27955: Make CodeGenCXX/arm-swiftcall.cpp tolerate C++11

probinson updated this revision to Diff 82021. probinson added a comment. Force C++03 on this test, to make it insensitive to future changes in the default dialect. https://reviews.llvm.org/D27955 Files: test/CodeGenCXX/arm-swiftcall.cpp Index: test/CodeGenCXX/arm-swiftcall.cpp ===

[PATCH] D27955: Make CodeGenCXX/arm-swiftcall.cpp tolerate C++11

This revision was automatically updated to reflect the committed changes. Closed by commit rL290145: Make another test insensitive to the default C++ dialect. (authored by probinson). Changed prior to commit: https://reviews.llvm.org/D27955?vs=82021&id=82028#toc Repository: rL LLVM https://

[PATCH] D27936: C++11 test cleanup: nonthrowing destructors

probinson added a reviewer: rjmccall. probinson added a comment. +rjmccall as IR Gen owner. https://reviews.llvm.org/D27936 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27994: Make two vtable tests tolerate C++11

probinson created this revision. probinson added a reviewer: rjmccall. probinson added a subscriber: cfe-commits. In C++11, we don't emit vtables as eagerly as we do for C++03, so fiddle the tests to emit them when the test expects them. In the C++11 test cleanup project, we're commonly making t

[PATCH] D27994: Make two vtable tests tolerate C++11

This revision was automatically updated to reflect the committed changes. Closed by commit rL290205: Make two vtable tests tolerate C++11. (authored by probinson). Changed prior to commit: https://reviews.llvm.org/D27994?vs=82126&id=82156#toc Repository: rL LLVM https://reviews.llvm.org/D27

[PATCH] D27936: C++11 test cleanup: nonthrowing destructors

This revision was automatically updated to reflect the committed changes. Closed by commit rL290207: C++11 test cleanup: nonthrowing destructors (authored by probinson). Changed prior to commit: https://reviews.llvm.org/D27936?vs=81982&id=82157#toc Repository: rL LLVM https://reviews.llvm.o

[PATCH] D27956: Make CodeGenCXX/stack-reuse-miscompile.cpp tolerate C++11

This revision was automatically updated to reflect the committed changes. Closed by commit rL290208: Make a test use a specific C++ dialect (authored by probinson). Changed prior to commit: https://reviews.llvm.org/D27956?vs=82020&id=82160#toc Repository: rL LLVM https://reviews.llvm.org/D2

[PATCH] D27794: Make some diagnostic tests C++11 clean

probinson marked an inline comment as done. probinson added a comment. FIXME added. https://reviews.llvm.org/D27794 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27794: Make some diagnostic tests C++11 clean

This revision was automatically updated to reflect the committed changes. Closed by commit rL290262: Make some diagnostic tests C++11 clean. (authored by probinson). Changed prior to commit: https://reviews.llvm.org/D27794?vs=81977&id=82248#toc Repository: rL LLVM https://reviews.llvm.org/D

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. Maybe instead, pass a flag to enable setting optnone on everything when the driver sees `-O0 -flto`? The patch as-is obviously has a massive testing cost, and it's easy to imagine people being tripped up by this in the future. https://reviews.llvm.org/D28404

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#638221, @mehdi_amini wrote: > In https://reviews.llvm.org/D28404#638217, @probinson wrote: > > > The patch as-is obviously has a massive testing cost, and it's easy to > > imagine people being tripped up by this in the future. > > > C

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#638350, @mehdi_amini wrote: > In https://reviews.llvm.org/D28404#638299, @probinson wrote: > > > In https://reviews.llvm.org/D28404#638221, @mehdi_amini wrote: > > > > > In https://reviews.llvm.org/D28404#638217, @probinson wrote: > >

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:962 + ShouldAddOptNone &= !D->hasAttr(); + ShouldAddOptNone &= !D->hasAttr(); + ShouldAddOptNone &= !F->hasFnAttribute(llvm::Attribute::AlwaysInline); chandlerc wrote: > why is op

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:896 + !CodeGenOpts.DisableO0ImplyOptNone && CodeGenOpts.OptimizationLevel == 0; + // We can't add optnone in the following cases, it won't pass the verifier + ShouldAddOptNone &= !D->hasAttr()

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:910-912 // OptimizeNone wins over OptimizeForSize and MinSize. F->removeFnAttr(llvm::Attribute::OptimizeForSize); F->removeFnAttr(llvm::Attribute::MinSize); mehdi_amin

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. Over the weekend I had a thought: Why is -O0 so special here? That is, after going to all this trouble to propagate -O0 to LTO, how does this generalize to propagating -O1 or any other specific -O option? (Maybe this question would be better dealt with on the dev l

<    1   2   3   4   5   6   >