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

probinson added a comment. In https://reviews.llvm.org/D28404#641557, @mehdi_amini wrote: > As I stand right now, there hasn't been any correction. > I still consider the fact that `optnone` wouldn't produce the "same" result > (modulo corner cases around `merging global variables` for instanc

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

probinson added a comment. In https://reviews.llvm.org/D28404#641606, @mehdi_amini wrote: > If we want to support `-O0 -flto` and `optnone` it the way to convey this to > the optimizer, I don't see the alternative. optsize != -Os (according to Chandler) minsize != -Oz (according to Chandler) o

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

probinson added a comment. I guess I'm getting irritated because people are trying to tell me what optnone means. I know what it means; I spent probably a whole year pushing to get it adopted. Optnone means: When you are running optimizations, try not to optimize this part, if you can. That'

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

probinson added a comment. In https://reviews.llvm.org/D28404#641757, @chandlerc wrote: > % ag OptimizeNone lib/Transforms/IPO > lib/Transforms/IPO/ForceFunctionAttrs.cpp > 47: .Case("optnone", Attribute::OptimizeNone) This is implementing a debugging option, not skipping a pass. >

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

probinson added a comment. @rsmith could you say whether it seems reasonable to have a LangOpts flag that basically means "`pragma clang optimize off` is always in effect." I think it would make the other optnone-related logic simpler. It would not be the only sort-of-codegen-related flag in

[PATCH] D28503: Documentation for the newly added x86 intrinsics.

probinson added inline comments. Comment at: emmintrin.h:1607 +/// +/// This intrinsic corresponds to the VMOVSD / MOVSD instruction. +/// should this be VMOVQ/MOVQ instead? Repository: rL LLVM https://reviews.llvm.org/D28503 ___

[PATCH] D28620: Guard __gnuc_va_list typedef

probinson created this revision. probinson added a reviewer: yaron.keren. probinson added a subscriber: cfe-commits. https://reviews.llvm.org/D28620 Files: lib/Headers/stdarg.h test/Headers/stdarg-gnuc_va_list.c Index: test/Headers/stdarg-gnuc_va_list.c =

[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] D14358: DWARF's forward decl of a template should have template parameters.

probinson updated this revision to Diff 106063. probinson added a comment. Refresh to current TOT, and ping. Funny what you can find in a year-old to-do list https://reviews.llvm.org/D14358 Files: lib/CodeGen/CGDebugInfo.cpp test/CodeGenCXX/debug-info-template-fwd-param.cpp test/Mod

[PATCH] D35583: Debug Info: Add a file: field to DIImportedEntity

probinson added a comment. Sweet. I don't have the chops to understand the Objective-C test but otherwise LGTM. Comment at: lib/Bitcode/Reader/MetadataLoader.cpp:1684 + HasFile ? getMDOrNull(Record[6]) : nullptr, + HasFile ? Rec

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

probinson created this revision. If a template instantiation uses a typedef'd name as a template parameter, this is usually resolved to its underlying type when we generate the name of the instance in the debug info. PS4 prefers to see the original parameter as in the source. Define an option

[PATCH] D42011: [DWARFv5] Enable MD5 checksums

probinson created this revision. probinson added reviewers: dblaikie, aprantl. probinson added a project: debug-info. Herald added subscribers: cfe-commits, JDevlieghere. Under `-gdwarf-5` generate MD5 checksums of source files to emit to the DWARF v5 line table. This consumes 16 bytes per sourc

[PATCH] D42011: [DWARFv5] Enable MD5 checksums

This revision was automatically updated to reflect the committed changes. Closed by commit rL322413: [DWARFv5] Have -gdwarf-5 generate MD5 checksums (authored by probinson, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D42011?vs=12970

[PATCH] D42011: [DWARFv5] Enable MD5 checksums

This revision was automatically updated to reflect the committed changes. Closed by commit rC322413: [DWARFv5] Have -gdwarf-5 generate MD5 checksums (authored by probinson, committed by ). Changed prior to commit: https://reviews.llvm.org/D42011?vs=129703&id=129708#toc Repository: rC Clang

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

probinson added a comment. Well, my understanding is that the pragma is a complete no-op even for MSVC, and is used only as a marker for editors such as Visual Studio's. So, unconditionally ignoring it would seem to be fine. https://reviews.llvm.org/D42248 ___

[PATCH] D42370: Issue local statics in correct DWARF lexical scope

probinson added subscribers: cfe-commits, probinson. probinson added a comment. +cfe-commits See also the LLVM change in https://reviews.llvm.org/D42369 and a debuginfo-tests test in https://reviews.llvm.org/D42371. https://reviews.llvm.org/D42370

[PATCH] D42371: Issue local statics in correct DWARF lexical scope

probinson added subscribers: cfe-commits, probinson. probinson added a comment. +cfe-commits See also the LLVM change in https://reviews.llvm.org/D42369 and Clang change in https://reviews.llvm.org/D42370. https://reviews.llvm.org/D42371 ___ cfe-co

[PATCH] D42581: [NVPTX] Emit debug info in DWARF-2 by default for Cuda devices.

probinson added a comment. If you want to force DWARF 2, probably clamping the version in LLVM would be simpler? Although most of the debug-info tests are architecture-specific and wouldn't run for an NVPTX target anyway. Repository: rC Clang https://reviews.llvm.org/D42581

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

probinson added a comment. In https://reviews.llvm.org/D42758#993936, @ruiu wrote: > > I also wonder which is better `#pragma comment(lib, "m")` or `#pragma > > comment(lib, "m")`. > > Sorry, I meant `#pragma comment(lib, "m")` or `#pragma comment("lib", "m")`. I can't swear to it but I don't

[PATCH] D42351: Emit DWARF "constructor" calling convention for every supported Clang CC

probinson added a comment. +1 for testcase. Repository: rC Clang https://reviews.llvm.org/D42351 ___ 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. 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] D28404: IRGen: Add optnone attribute on function during O0

probinson added a comment. In https://reviews.llvm.org/D28404#675687, @chandlerc wrote: > In https://reviews.llvm.org/D28404#675616, @mehdi_amini wrote: > > > We're still waiting for @rsmith to comment whether it'd be better to `have > > a LangOpts flag that basically means "pragma clang optimiz

[PATCH] D27549: [DebugInfo] Add support for __fp16, float, and double constants.

probinson added subscribers: cfe-commits, probinson. probinson added a comment. Hi David! As this is a Clang patch, you should subscribe cfe-commits rather than llvm-commits. I've done that for you. See also inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:3765 +I

[PATCH] D27549: [DebugInfo] Add support for __fp16, float, and double constants.

probinson accepted this revision. probinson added a reviewer: probinson. probinson added a comment. This revision is now accepted and ready to land. LGTM. Comment at: lib/CodeGen/CGDebugInfo.cpp:3765 +InitExpr = + DBuilder.createConstantValueExpression(Init.getFloat

[PATCH] D27597: [DebugInfo] Restore test case for long double constants.

probinson added a comment. As dblaikie said in email, probably better to make this X86-specific; if long-double varies by OS you can put in a specific triple. FTR we don't rely on Perl being available everywhere, anything that does this kind of scripty stuff uses Python. Com

[PATCH] D27641: DebugInfo: Added support for Checksum debug info feature (Clang part)

probinson added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:350 + std::string Checksum; + SM.getChecksumMD5(SM.getFileID(Loc), Checksum); + rnk wrote: > We should only do this if `CGM.getCodeGenOpts().EmitCodeView`, or we will > regress compile ti

[PATCH] D27597: [DebugInfo] Restore test case for long double constants.

probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D27597#621618, @dgross wrote: > So would a Python equivalent of the Perl be acceptable? I think this is an > academic question -- better to explicitly test m

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

probinson added a comment. > I think we should assume that PS4 will as well. Yes, please. We are essentially locked to the ABI as of clang 3.2. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117616/new/ https://reviews.llvm.org/D117616 __

[PATCH] D120066: [NFC] Fix debug-info-hotpatch.cpp failure due to downstream regex issue.

probinson added a comment. Seems to me there have been other cases where `.*` matching was greedier than expected, so this seems like the right fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120066/new/ https://reviews.llvm.org/D120066 _

[PATCH] D111457: [test] Add lit helper for windows paths

probinson added inline comments. Comment at: llvm/utils/lit/lit/TestRunner.py:1124 +if kIsWindows: +fs_root = 'C:\\' +substitutions.extend([ rnk wrote: > It is pretty common to run the LLVM test suite on secondary drives, > especially on buildbot

[PATCH] D119040: Fix LookupTest where it was missing an assertion

probinson added a reviewer: hokein. probinson added a subscriber: hokein. probinson added a comment. Herald added a project: All. + @hokein who has done work in the one place where `replaceNestedName` is used. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119040/new/ https://reviews.ll

[PATCH] D101259: [clang-tidy] Fix cppcoreguidelines-pro-type-vararg false positives with __builtin_ms_va_list

probinson added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp:7 + +// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-vararg %t + TWeaver wrote: > njames93 wrote: > > TWeaver wrote: > > > Is

[PATCH] D101259: [clang-tidy] Fix cppcoreguidelines-pro-type-vararg false positives with __builtin_ms_va_list

probinson added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp:7 + +// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-vararg %t + probinson wrote: > TWeaver wrote: > > njames93 wrote: > > >

[PATCH] D99409: [clang] Speedup line offset mapping computation

probinson added inline comments. Comment at: clang/lib/Basic/SourceManager.cpp:1255 -#ifdef __SSE2__ -#include -#endif +// Check if mutli-byte word x has bytes between m and n, included. This may also +// catch bytes equal to n + 1. Typo: multi-byte. Also, I

[PATCH] D98873: Document -fcrash-diagnostics-dir

probinson created this revision. probinson added a reviewer: hans. Herald added subscribers: jansvoboda11, dang. probinson requested review of this revision. Herald added a project: clang. This was added in LLVM 7.0 but without help text or other docs. Repository: rG LLVM Github Monorepo http

[PATCH] D98514: [RGT] Fix ASTMatchersTest so all assertions are executed

probinson added a reviewer: gribozavr. probinson added a comment. +gribozavr who has been in this file most recently Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98514/new/ https://reviews.llvm.org/D98514 _

[PATCH] D98873: Document -fcrash-diagnostics-dir

This revision was automatically updated to reflect the committed changes. Closed by commit rGe150be612bf7: Document -fcrash-diagnostics-dir (authored by probinson). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98873/new/ https://reviews.llvm.org/D9

[PATCH] D99199: Make -fcrash-diagnostics-dir control the Windows (mini-)dump location

probinson created this revision. probinson added a reviewer: rnk. Herald added subscribers: dexonsmith, hiraditya. probinson requested review of this revision. Herald added projects: clang, LLVM. There's no automated test for this, as I don't see any existing tests for creating minidumps. I have

[PATCH] D99250: [DebugInfo] Fix the mismatching of C++ language tags and Dwarf versions.

probinson added a comment. In D99250#2649507 , @Esme wrote: > Thx! @aprantl The motivation of the patch came from the crash of tag name > mismatching when using DBX under AIX. And modifying the debugger doesn't seem > to make sense? A consumer should no

[PATCH] D99250: [DebugInfo] Fix the mismatching of C++ language tags and Dwarf versions.

probinson added a comment. I think there are degrees of compatibility with regard to DWARF, which is inherently supposed to be extensible if the extension can be safely ignored by a consumer. This is the "permissive" rule in the standard. - A new FORM must *never* be used, because that makes i

[PATCH] D99250: [DebugInfo] Fix the mismatching of C++ language tags and Dwarf versions.

probinson added a comment. In D99250#2651394 , @jsji wrote: > The list is growing, but sure, we will post a thread in llvm-dev about what > we met so far. > Two big one would be that DBX not supporting string section(DW_FORM_strp) and > column-info i

[PATCH] D98514: [RGT] Fix ASTMatchersTest so all assertions are executed

probinson abandoned this revision. probinson added a comment. Problem has gone away, test assertions are now executed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98514/new/ https://reviews.llvm.org/D98514 ___

[PATCH] D99250: [DebugInfo] Fix the mismatching of C++ language tags and Dwarf versions.

probinson added a comment. In D99250#2651953 , @probinson wrote: > In D99250#2651204 , @dblaikie wrote: > >> Does anyone else have a DWARFv3 consumer they care about? (@aprantl and >> @probinson) >> >> Does anyone

[PATCH] D99199: Make -fcrash-diagnostics-dir control the Windows (mini-)dump location

probinson updated this revision to Diff 333884. probinson added a comment. Remove ifdefs, tweak descriptions/comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99199/new/ https://reviews.llvm.org/D99199 Files: clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/crash-diagn

[PATCH] D99199: Make -fcrash-diagnostics-dir control the Windows (mini-)dump location

probinson marked 2 inline comments as done. probinson added a comment. Addressed comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99199/new/ https://reviews.llvm.org/D99199 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D99250: [DebugInfo] Fix the mismatching of C++ language tags and Dwarf versions.

probinson added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:572 LangTag = llvm::dwarf::DW_LANG_C_plus_plus_14; -else if (LO.CPlusPlus11) +else if (LO.CPlusPlus11 && CGM.getCodeGenOpts().DwarfVersion >= 5) LangTag = llvm::dwarf::DW_LANG_C_p

[PATCH] D99160: [X86][FastISEL] Support DW_TAG_call_site_parameter with FastISEL

probinson added a comment. FastISel is normally used only at -O0, I wouldn't expect any parameters to be optimized out at -O0. The test is running llc with default optimization, which is -O2, and forcing fast-isel; this is not a usual combination and I wouldn't expect us to spend any effort mak

[PATCH] D99238: [DebugInfo] Enable the call site parameter feature by default

probinson added a reviewer: dblaikie. probinson added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1648 - if (Opts.OptimizationLevel > 0 && Opts.hasReducedDebugInfo() && + if (Opts.hasReducedDebugInfo() && llvm::is_contained(DebugEntryValueArc

[PATCH] D99199: Make -fcrash-diagnostics-dir control the Windows (mini-)dump location

probinson added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99199/new/ https://reviews.llvm.org/D99199 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99199: Make -fcrash-diagnostics-dir control the Windows (mini-)dump location

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG04b3c8c52c54: Pass -fcrash-diagnostics-dir along to LLVM (authored by probinson). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D100049: Remove .gitignore entries not relevant in the monorepo.

This revision was automatically updated to reflect the committed changes. Closed by commit rG676a9ab5e406: Remove .gitignore entries not relevant in the monorepo. (authored by probinson). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

[PATCH] D100298: [Clang][Coroutine][DebugInfo] Follow-up: reduce a tests ordering requirements

probinson added a comment. I am not 100% convinced this has the desired effect. I believe what it does is: - Search for the first occurrence of each of "val" "moParam" and "mcParam" - After the last of the above occurrences, ensure there are no other occurrences of "val" "moParam" or "mcParam"

[PATCH] D100298: [Clang][Coroutine][DebugInfo] Follow-up: reduce a tests ordering requirements

probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. One optional nit and LGTM, assuming the DILocalVariable ordering is consistent with what we see downstream. Comment at: clang/test/CodeGenCoroutines/coro-dwarf.cpp:1 -

[PATCH] D100630: [Debug-Info][DBX] DW_TAG_rvalue_reference_type should not be generated when dwarf version is smaller than 4

probinson added a comment. If DBX is going to be really pedantic about not recognizing tags or attributes that don't align with the DWARF version, maybe we're better off with really supporting `-gstrict-dwarf` and just have DBX tuning imply that. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D100826: [Debug-Info][NFC] add -gstrict-dwarf support in backend

probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. You have a clang-format warning, aside from that LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100826/new/ https://reviews.llvm.org/

[PATCH] D100809: [Debug-Info] implement -gstrict-dwarf

probinson added a comment. Nit on the description, this patch supports -gstrict-dwarf in the frontend. Comment at: clang/docs/ClangCommandLineReference.rst:3549-3550 +Use DWARF extensions in later DWARF versions. + .. option:: -gz=, -gz (equivalent to -gz=zlib) -

[PATCH] D111352: [clang] Fix absolute file paths with -fdebug-prefix-map

probinson accepted this revision. probinson added a comment. This revision is now accepted and ready to land. I suspect this is not the only way for directories and filenames to become confused, but it certainly helps. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

<    1   2   3   4   5   6