Page MenuHomePhabricator

probinson (Paul Robinson)
User

Projects

User Details

User Since
May 9 2013, 11:10 AM (314 w, 5 d)

Recent Activity

Today

probinson added a comment to D62118: [DWARF] hoist nullptr checks. NFC.

Is there a test case for this codepath? Or should it be an assertion instead?

Tue, May 21, 12:20 PM · Restricted Project
probinson added a comment to D62202: Work around a Visual C++ bug.

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.

Tue, May 21, 11:30 AM · Restricted Project
probinson added a comment to D62202: Work around a Visual C++ bug.

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) - https://llvm.org/docs/CodingStandards.html#anonymous-namespaces

Tue, May 21, 9:08 AM · Restricted Project
probinson added a comment to D62202: Work around a Visual C++ bug.

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 Studio 2015 (some specific update), which assumes all Visual Studio 2017 versions are usable. So by that criterion, we need this patch for all Visual Studio 2017 versions to be usable.
Arguably the anonymous-namespace version is the "more modern" tactic anyway, so as a long-term fix this is actually fine anyway.

Tue, May 21, 8:35 AM · Restricted Project
probinson created D62202: Work around a Visual C++ bug.
Tue, May 21, 8:17 AM · Restricted Project
probinson committed rG35a2196bd116: Fix typo in r361251. (authored by probinson).
Fix typo in r361251.
Tue, May 21, 6:24 AM
probinson committed rL361256: Fix typo in r361251..
Fix typo in r361251.
Tue, May 21, 6:20 AM
probinson committed rG0a16ba856bea: [DebugInfo] Fix tests missed by r362148 (authored by probinson).
[DebugInfo] Fix tests missed by r362148
Tue, May 21, 5:47 AM
probinson committed rL361251: [DebugInfo] Fix tests missed by r362148.
[DebugInfo] Fix tests missed by r362148
Tue, May 21, 5:47 AM
probinson committed rG9d5351cab69a: De-Window-ize a test (authored by probinson).
De-Window-ize a test
Tue, May 21, 5:07 AM
probinson committed rL361250: De-Window-ize a test.
De-Window-ize a test
Tue, May 21, 5:06 AM
probinson committed rG9c5632693426: [DebugInfo] Handle '# line "file"' correctly for asm source. This provides the… (authored by probinson).
[DebugInfo] Handle '# line "file"' correctly for asm source. This provides the…
Tue, May 21, 4:57 AM
probinson committed rL361248: [DebugInfo] Handle '# line "file"' correctly for asm source..
[DebugInfo] Handle '# line "file"' correctly for asm source.
Tue, May 21, 4:56 AM
probinson closed D62074: [DebugInfo] Handle '# line "file"' correctly for asm source..
Tue, May 21, 4:56 AM · Restricted Project, debug-info
probinson committed rG116e8d4876f9: [DebugInfo] Handle -main-file-name correctly for asm source. This option… (authored by probinson).
[DebugInfo] Handle -main-file-name correctly for asm source. This option…
Tue, May 21, 4:51 AM
probinson committed rL361245: [DebugInfo] Handle -main-file-name correctly for asm source..
[DebugInfo] Handle -main-file-name correctly for asm source.
Tue, May 21, 4:50 AM
probinson closed D62071: [DebugInfo] Handle -main-file-name correctly for asm source..
Tue, May 21, 4:50 AM · Restricted Project, debug-info

Yesterday

probinson committed rGd90193695751: Fix test not to use UNSUPPORTED as a FileCheck prefix. It was not causing a… (authored by probinson).
Fix test not to use UNSUPPORTED as a FileCheck prefix. It was not causing a…
Mon, May 20, 7:58 AM
probinson committed rC361161: Fix test not to use UNSUPPORTED as a FileCheck prefix..
Fix test not to use UNSUPPORTED as a FileCheck prefix.
Mon, May 20, 7:58 AM
probinson committed rL361161: Fix test not to use UNSUPPORTED as a FileCheck prefix..
Fix test not to use UNSUPPORTED as a FileCheck prefix.
Mon, May 20, 7:58 AM
probinson accepted D60283: [DebugInfo] Don't emit checksums when compiling a preprocessed CPP.

LGTM

Mon, May 20, 6:46 AM · Restricted Project

Fri, May 17

probinson created D62074: [DebugInfo] Handle '# line "file"' correctly for asm source..
Fri, May 17, 12:18 PM · Restricted Project, debug-info
probinson created D62071: [DebugInfo] Handle -main-file-name correctly for asm source..
Fri, May 17, 11:15 AM · Restricted Project, debug-info
probinson added a comment to D60386: FileCheck [6/12]: Introduce numeric variable definition.

That's a great idea, especially the "string variable" and "string substitution". I was not pleased with the name either but the only thing I thought of was regex variable which is plain wrong, I completely missed the obvious.

Naming is hard, I clearly didn't think of it right away either.

Fri, May 17, 8:16 AM · Restricted Project

Thu, May 16

probinson added a comment to D60386: FileCheck [6/12]: Introduce numeric variable definition.

There's still some inconsistency in the terminology, as clearly evidenced by the name and description of the IsNumExpr member of FileCheckPatternSubstitution. That class used to be where we'd find variables (now called pattern variables); but it has had numeric expressions crammed into it, except the comment wants to call it a substitution, and so we have "pattern" and "substitution" and "expression" all competing for clarity.

Thu, May 16, 7:29 AM · Restricted Project

Wed, May 15

probinson added a comment to D60283: [DebugInfo] Don't emit checksums when compiling a preprocessed CPP.

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.

Wed, May 15, 9:50 AM · Restricted Project
probinson added a comment to D61584: [DebugInfo] Some fields do not need relocations even relax is enabled..

It still bothers me that MCDwarf has to know exactly when to override the target's decision to do relaxation.

Wed, May 15, 9:08 AM · Restricted Project

Tue, May 14

probinson added inline comments to D60386: FileCheck [6/12]: Introduce numeric variable definition.
Tue, May 14, 10:09 AM · Restricted Project
probinson added a comment to D60831: [DebugInfo@O2][LoopVectorize] pr39024: Vectorized code linenos step through loop even after completion.

It sounds like inlining a function with a loop has a bug with respect to how the loop metadata is handled, and your patch merely tripped over that. Would it be reasonable to fix the inlining-loop-metadata bug separately first? And then the original patch is likely to Just Work?
Fixing one bug at a time is more in line with project practices.

Tue, May 14, 8:40 AM · debug-info, Restricted Project
probinson committed rGac2f5a61be2a: Replace lit feature keyword 'not_COFF' with 'uses_COFF'. (authored by probinson).
Replace lit feature keyword 'not_COFF' with 'uses_COFF'.
Tue, May 14, 7:50 AM
probinson committed rL360680: Replace lit feature keyword 'not_COFF' with 'uses_COFF'..
Replace lit feature keyword 'not_COFF' with 'uses_COFF'.
Tue, May 14, 7:50 AM
probinson closed D61791: Replace lit feature keyword 'non_COFF' with 'uses_COFF'..
Tue, May 14, 7:50 AM · Restricted Project
probinson accepted D61891: DWARF v5: emit DW_AT_addr_base if DW_AT_low_pc references .debug_addr.

LGTM once you rephrase the comment in the test.

Tue, May 14, 7:05 AM · Restricted Project
probinson accepted D61184: [Salvage] Change salvage debug info implementation to use new DW_OP_LLVM_convert where needed.

LGTM after you add the full-stop noted below.

Tue, May 14, 6:52 AM · Restricted Project

Mon, May 13

probinson added inline comments to D61184: [Salvage] Change salvage debug info implementation to use new DW_OP_LLVM_convert where needed.
Mon, May 13, 10:35 AM · Restricted Project
probinson added a comment to D60386: FileCheck [6/12]: Introduce numeric variable definition.

Starting set of comments on this one. Haven't looked at everything yet.

Mon, May 13, 10:22 AM · Restricted Project
probinson committed rGb38e4b28e399: Stop defining negative versions of some lit feature keywords: zlib/nozlib… (authored by probinson).
Stop defining negative versions of some lit feature keywords: zlib/nozlib…
Mon, May 13, 10:17 AM
probinson committed rL360603: Stop defining negative versions of some lit feature keywords:.
Stop defining negative versions of some lit feature keywords:
Mon, May 13, 10:16 AM

Fri, May 10

probinson committed rG8273fdc2a484: Replace 'REQUIRES: nozlib' with '!zlib' because we don't need two ways to say… (authored by probinson).
Replace 'REQUIRES: nozlib' with '!zlib' because we don't need two ways to say…
Fri, May 10, 11:47 AM
probinson committed rL360455: Replace 'REQUIRES: nozlib' with '!zlib' because we don't need two ways.
Replace 'REQUIRES: nozlib' with '!zlib' because we don't need two ways
Fri, May 10, 11:46 AM
probinson committed rG484603488118: Replace 'REQUIRES: nozlib' with '!zlib' because we don't need two ways to say… (authored by probinson).
Replace 'REQUIRES: nozlib' with '!zlib' because we don't need two ways to say…
Fri, May 10, 11:34 AM
probinson committed rC360452: Replace 'REQUIRES: nozlib' with '!zlib' because we don't need two ways.
Replace 'REQUIRES: nozlib' with '!zlib' because we don't need two ways
Fri, May 10, 11:34 AM
probinson committed rL360452: Replace 'REQUIRES: nozlib' with '!zlib' because we don't need two ways.
Replace 'REQUIRES: nozlib' with '!zlib' because we don't need two ways
Fri, May 10, 11:34 AM
probinson committed rG0c55985bbb40: Replace 'REQUIRES: not_?san' with 'UNSUPPORTED: ?san' as that better expresses… (authored by probinson).
Replace 'REQUIRES: not_?san' with 'UNSUPPORTED: ?san' as that better expresses…
Fri, May 10, 11:08 AM
probinson committed rL360449: Replace 'REQUIRES: not_?san' with 'UNSUPPORTED: ?san' as that better.
Replace 'REQUIRES: not_?san' with 'UNSUPPORTED: ?san' as that better
Fri, May 10, 11:05 AM
probinson committed rGd5d4df98bb9e: Replace 'REQUIRES: not_?san' with 'UNSUPPORTED: ?san' as that better expresses… (authored by probinson).
Replace 'REQUIRES: not_?san' with 'UNSUPPORTED: ?san' as that better expresses…
Fri, May 10, 10:57 AM
probinson committed rL360447: Replace 'REQUIRES: not_?san' with 'UNSUPPORTED: ?san' as that better.
Replace 'REQUIRES: not_?san' with 'UNSUPPORTED: ?san' as that better
Fri, May 10, 10:54 AM
probinson committed rC360447: Replace 'REQUIRES: not_?san' with 'UNSUPPORTED: ?san' as that better.
Replace 'REQUIRES: not_?san' with 'UNSUPPORTED: ?san' as that better
Fri, May 10, 10:54 AM
probinson created D61791: Replace lit feature keyword 'non_COFF' with 'uses_COFF'..
Fri, May 10, 8:54 AM · Restricted Project
probinson committed rG4b66e0fd47c9: Replace lit feature keyword 'non-ms-sdk' with 'ms-sdk'; eliminate 'non-ps4-sdk'… (authored by probinson).
Replace lit feature keyword 'non-ms-sdk' with 'ms-sdk'; eliminate 'non-ps4-sdk'…
Fri, May 10, 6:42 AM
probinson committed rC360425: Replace lit feature keyword 'non-ms-sdk' with 'ms-sdk'; eliminate.
Replace lit feature keyword 'non-ms-sdk' with 'ms-sdk'; eliminate
Fri, May 10, 6:38 AM
probinson committed rL360425: Replace lit feature keyword 'non-ms-sdk' with 'ms-sdk'; eliminate.
Replace lit feature keyword 'non-ms-sdk' with 'ms-sdk'; eliminate
Fri, May 10, 6:37 AM
probinson accepted D61680: [X86] Avoid SFB - Fix inconsistent codegen with/without debug info .

LGTM

Fri, May 10, 4:55 AM · Restricted Project

Thu, May 9

probinson abandoned D61752: Re-enable a test for non-Windows.

@stella.stamenova fixed this using system-windows instead.

Thu, May 9, 12:49 PM · Restricted Project
probinson created D61752: Re-enable a test for non-Windows.
Thu, May 9, 12:24 PM · Restricted Project
probinson added a comment to D61611: [JITLoaderGDB] Set eTypeJIT for objects read from JIT descriptors.

@stella.stamenova I'm not familiar with any lit feature that gives a special meaning to the prefix "no". The opposite of "REQUIRES: windows" is not "REQUIRES: nowindows" but "UNSUPPORTED: windows" AFAIK.
This part of the discussion should probably be taken to llvm-dev, though.

FTR I don't see that lldb's lit.cfg.py sets any features based on host OS, so even "UNSUPPORTED: windows" probably does not work currently.

@probinson LLDB's lit configuration derives from the lit configuration in LLVM. The most important file you are looking for is:

llvm\utils\lit\lit\llvm\config.py

You can see there that we add various platform to the list of available features including system-windows, so XFAIL: system-windows, UNSUPPORTED: system-windows, etc. all work.

This is also where binary_features are set, such as 'zlib/nozlib', 'asan/not_asan' etc. The prefix varies for historical reasons, but the paradigm has existed for a long time. If we wanted to support nosystem-windows, we would add it here.

Thu, May 9, 11:06 AM · Restricted Project, Restricted Project
probinson added inline comments to D61680: [X86] Avoid SFB - Fix inconsistent codegen with/without debug info .
Thu, May 9, 10:07 AM · Restricted Project
probinson committed rG0b68fc3f59b5: Re-enable lit test shtest-timeout.py on non-Windows. (authored by probinson).
Re-enable lit test shtest-timeout.py on non-Windows.
Thu, May 9, 10:00 AM
probinson committed rL360356: Re-enable lit test shtest-timeout.py on non-Windows..
Re-enable lit test shtest-timeout.py on non-Windows.
Thu, May 9, 10:00 AM
probinson added a comment to D61611: [JITLoaderGDB] Set eTypeJIT for objects read from JIT descriptors.

@stella.stamenova I'm not familiar with any lit feature that gives a special meaning to the prefix "no". The opposite of "REQUIRES: windows" is not "REQUIRES: nowindows" but "UNSUPPORTED: windows" AFAIK.
This part of the discussion should probably be taken to llvm-dev, though.

Thu, May 9, 9:46 AM · Restricted Project, Restricted Project
probinson added a comment to D61611: [JITLoaderGDB] Set eTypeJIT for objects read from JIT descriptors.

@stella.stamenova Can you have a look at the lit test please? It works on macOS and Linux, but I didn't test Windows. Should I add something like # REQUIRES: nowindows or is it fine like this?

Thu, May 9, 8:13 AM · Restricted Project, Restricted Project
probinson added a comment to D60385: FileCheck [5/12]: Introduce regular numeric variables.

I am pretty happy with this now, maybe give the other people who have been participating a day or so to chime in again.

Thu, May 9, 7:20 AM · Restricted Project

Wed, May 8

probinson added a comment to D61445: [FileCheck] Fix code style of method comments.

Still good.

Wed, May 8, 2:20 PM · Restricted Project
probinson accepted D61445: [FileCheck] Fix code style of method comments.

One or two last nits and LGTM.

Wed, May 8, 11:34 AM · Restricted Project
probinson added a comment to D61680: [X86] Avoid SFB - Fix inconsistent codegen with/without debug info .

I wonder if this should be an MIR test? Fewer moving parts.

Wed, May 8, 11:21 AM · Restricted Project
probinson added a comment to D61445: [FileCheck] Fix code style of method comments.

Sorry, I should have been more thorough about this previously: My understanding is that if a function description mentions any parameters with \p, then it needs to mention all of them.
For some of the one-liner descriptions, if you prefer to just remove the \p that's okay with me. You've been suffering a lot through this process and I don't want to create more work than we really need to!

Wed, May 8, 9:51 AM · Restricted Project
probinson accepted D61679: [FileCheck, NFC] Split defines.txt in two.

Fix the one typo and LGTM.

Wed, May 8, 9:30 AM · Restricted Project
probinson added a comment to D61680: [X86] Avoid SFB - Fix inconsistent codegen with/without debug info .

The point is not that a certain set of moves comes out, but that the *same* set of moves comes out regardless of debug info.
So, the test wants to have two copies of the same function, one with debug instructions and one without; and both functions should produce the same sequence of moves.

Wed, May 8, 9:07 AM · Restricted Project
probinson requested changes to D61679: [FileCheck, NFC] Split defines.txt in two.

Aha. The new versions add the word "pattern" to some of the diagnostics, in pattern-defines-diagnostics.txt. That would be the functional change belonging to D60385, which means that test should not pass with an unmodified FileCheck? Please verify.

Wed, May 8, 8:56 AM · Restricted Project
probinson added a comment to D60385: FileCheck [5/12]: Introduce regular numeric variables.

I still can't tell what the functional difference is, in pattern-defines.txt. Everything else looks fine.

There is none, pattern-defines + pattern-defines-diagnostics = old defines.txt + some new comments, as per James' comments.

Wed, May 8, 5:23 AM · Restricted Project

Tue, May 7

probinson added inline comments to D60385: FileCheck [5/12]: Introduce regular numeric variables.
Tue, May 7, 11:01 AM · Restricted Project
probinson added a comment to D60385: FileCheck [5/12]: Introduce regular numeric variables.

I still can't tell what the functional difference is, in pattern-defines.txt. Everything else looks fine.

Tue, May 7, 8:54 AM · Restricted Project
probinson added a comment to D61584: [DebugInfo] Some fields do not need relocations even relax is enabled..

Thanks for the reference, that helps some.
However I still think this isn't the right way to go about fixing the problem. This started because RISC-V decided not to resolve certain kinds of expressions, and then it turns out that applies to too many expressions. Somebody suggested that the asm backend decision should be more refined, based on symbol section perhaps, and that feels like a more principled way to go about it.

Tue, May 7, 8:35 AM · Restricted Project
probinson added a comment to D61625: Debug Info: Support address space attributes on rvalue references..

+1 to Bjorn's comment.
The DWARF spec says address class is also allowed on a "subroutine or subroutine type" do we allow generating code into nonzero address spaces?

Tue, May 7, 7:47 AM · Restricted Project, debug-info

Mon, May 6

probinson added a comment to D60385: FileCheck [5/12]: Introduce regular numeric variables.

Nearly there! One final grammar nit and some test comments.

Mon, May 6, 2:24 PM · Restricted Project
probinson committed rG1e18bfe89213: Fix more Windows bots after r360015. Depending on the environment, the… (authored by probinson).
Fix more Windows bots after r360015. Depending on the environment, the…
Mon, May 6, 12:13 PM
probinson committed rL360065: Fix more Windows bots after r360015..
Fix more Windows bots after r360015.
Mon, May 6, 12:10 PM
probinson added a comment to D61584: [DebugInfo] Some fields do not need relocations even relax is enabled..

I don't claim to be an MCExpr expert, but I think this should not be necessary. There are other situations where symbol differences are computed and emitted as constants, for example when DW_AT_high_pc is a length rather than an address, and they clearly don't require this tactic.

Mon, May 6, 6:09 AM · Restricted Project

Fri, May 3

probinson added a comment to D61496: Fixed tests where grep was not matching the linefeed.

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 dos2unix to the RUN lines is the wrong fix, either way.

Fri, May 3, 6:18 AM · Restricted Project

Thu, May 2

probinson accepted D61445: [FileCheck] Fix code style of method comments.

Thanks! Some small things, and LGTM once you've fixed those.

Thu, May 2, 9:39 AM · Restricted Project
probinson added inline comments to D61432: Non-8-bit bytes showcase.
Thu, May 2, 8:15 AM · Restricted Project

Wed, May 1

probinson added a comment to D60385: FileCheck [5/12]: Introduce regular numeric variables.

Haven't looked at the tests yet.

Wed, May 1, 2:14 PM · Restricted Project
probinson accepted D60384: FileCheck [4/12]: Introduce @LINE numeric expressions.

LGTM

Wed, May 1, 11:49 AM · Restricted Project
probinson added a comment to D60384: FileCheck [4/12]: Introduce @LINE numeric expressions.

Another round of commentary nits, plus a couple more substantive remarks. If you accept them all, LGTM.

Wed, May 1, 9:39 AM · Restricted Project
probinson added a comment to D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools..

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.py: C:/j/w/opensource/opensource_to_sce_merge__2/o/llvm\utils\lit\lit\llvm\config.py:336: fatal: couldn't find 'clang' program, try setting CLANG in your environment
C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\V140\Microsoft.CppCommon.targets(171,5): error MSB6006: "cmd.exe" exited with code 2. [C:\j\w\opensource\opensource_to_sce_merge__2\build\check-all.vcxproj]
Wed, May 1, 9:07 AM · Restricted Project, Restricted Project

Tue, Apr 30

probinson added a comment to D60384: FileCheck [4/12]: Introduce @LINE numeric expressions.

First pass looking at docs and commentary.

Tue, Apr 30, 6:07 AM · Restricted Project

Fri, Apr 26

probinson added a comment to D60283: [DebugInfo] Don't emit checksums when compiling a preprocessed CPP.

Thanks Paul, your solution is even better. I'll apply rL333311 locally - if everything's fine, do you mind if I re-land it again?

Fri, Apr 26, 11:49 AM · Restricted Project
probinson added a comment to D60283: [DebugInfo] Don't emit checksums when compiling a preprocessed CPP.

I had tried to do this in r333311 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 flag to PresumedLoc?
What I did was something like this, in computeChecksum():

const SrcMgr::SLocEntry &Entry = SM.getSLocEntry(FID, &Invalid);
if (Invalid || !Entry.isFile() || Entry.getFile().hasLineDirectives())
  return None;
Fri, Apr 26, 8:32 AM · Restricted Project

Thu, Apr 25

probinson added a comment to D58033: Add option for emitting dbg info for call site parameters.

I'm okay with this, but give @aprantl a chance to confirm.

Thu, Apr 25, 12:41 PM · debug-info
probinson added a comment to D60274: [ELF] Implement Dependent Libraries Feature.

I'm okay with the PS4-specific bits.

Thu, Apr 25, 7:46 AM · Restricted Project

Wed, Apr 24

probinson added inline comments to D58033: Add option for emitting dbg info for call site parameters.
Wed, Apr 24, 8:04 AM · debug-info
probinson accepted D60382: FileCheck [2/12]: Stricter parsing of -D option.

LGTM after the comment on the parseVariable loop is addressed.

Wed, Apr 24, 7:50 AM · Restricted Project
probinson accepted D60383: FileCheck [3/12]: Stricter parsing of @LINE expressions.

LGTM

Wed, Apr 24, 7:48 AM · Restricted Project
probinson added inline comments to D60382: FileCheck [2/12]: Stricter parsing of -D option.
Wed, Apr 24, 7:40 AM · Restricted Project
probinson added a comment to D60831: [DebugInfo@O2][LoopVectorize] pr39024: Vectorized code linenos step through loop even after completion.

This will be my last comment on the topic and then everyone can get back to work :-). I agree with the LLVM IR requirement that one not to lie to application developers; lies are worse than nothing. However, I don't feel that you have adequately explained why attributing to any of the available (file, line) mappings for an instruction that has been merged is incorrect and not 100% reliable. Just because we can't include all contributing mappings for a merged instruction doesn't make any one unreliable; I see attributing to any one of a set of mappings as 100% correct, even though it is only partial information.

Wed, Apr 24, 7:08 AM · debug-info, Restricted Project

Tue, Apr 23

probinson accepted D60896: Improve error reporting for mismatched value sites in IPGO.

LGTM. The previous diagnostic would be useful only to compiler developers.

Tue, Apr 23, 9:06 AM · Restricted Project

Apr 18 2019

probinson added a comment to D58033: Add option for emitting dbg info for call site parameters.

@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 (like we pushed here), since LLDB doesn't read this type of debug info (yet).
The plan is, when LLDB has all necessary info and we are sure that everything works fine during using this info in debugging sessions, we can turn it to ON by default.

Does that make sense ? :)

Apr 18 2019, 7:36 AM · debug-info

Apr 16 2019

probinson added inline comments to D60382: FileCheck [2/12]: Stricter parsing of -D option.
Apr 16 2019, 8:51 AM · Restricted Project
probinson added a comment to D58033: Add option for emitting dbg info for call site parameters.

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.

Apr 16 2019, 8:45 AM · debug-info
probinson added a comment to D60382: FileCheck [2/12]: Stricter parsing of -D option.

Therefore, if we want to remove the diagnostic from that function we need to remove the ability to define a numeric variable from a numeric expression (which allows stuff like -D#NUMVAR1=10 -D#NUMVAR2=NUMVAR1+4. How useful do you think this ability is?

Apr 16 2019, 7:32 AM · Restricted Project