Page MenuHomePhabricator

probinson (Paul Robinson)
User

Projects

User Details

User Since
May 9 2013, 11:10 AM (297 w, 6 d)

Recent Activity

Today

probinson added a comment to D56789: Allow CHECK-SAME, NEXT and EMPTY after CHECK-DAG.

The revision title and summary contradict each other. The title says to allow something, but the summary says that already works; then the summary says CHECK-DAG can't be first, while the check-dag.txt test clearly already has CHECK-DAG as the first directive.
Please fix up the title and summary so that they correctly describe what you are trying to accomplish. This will help reviewers to make sure the patch does what you want.

Wed, Jan 23, 11:44 AM

Fri, Jan 18

probinson added a comment to D56767: [AsmPrinter] Collapse .loc 0 0 directives.

Thanks! I'm not surprised I didn't get it completely right; it was such a maze of twisty passages all different.

Fri, Jan 18, 11:47 AM

Mon, Jan 14

probinson accepted D55825: [FileCheck] Suppress old -v/-vv diags if dumping input.

LGTM although I do wonder at the usefulness of the wildcards in the implicit-not checks.

Mon, Jan 14, 1:42 PM
probinson accepted D54465: [CodeGen] Fix bugs in LiveDebugVariables when debug labels are generated..

LGTM

Mon, Jan 14, 8:04 AM · debug-info

Thu, Jan 10

probinson added a comment to D55940: Detect incorrect FileCheck variable CLI definition.

I'm happy.

Thu, Jan 10, 11:15 AM
probinson accepted D56549: Add support for prefix-only CLI options.

Nice work on the prefix tests. One comment nit and LGTM.

Thu, Jan 10, 11:13 AM
probinson added a comment to D55940: Detect incorrect FileCheck variable CLI definition.

A couple of comment phrasing nits.

Thu, Jan 10, 7:54 AM

Wed, Jan 9

probinson added inline comments to D55825: [FileCheck] Suppress old -v/-vv diags if dumping input.
Wed, Jan 9, 7:25 AM
probinson added a comment to D55781: Make CC mangling conditional on the ABI version.

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.

Wed, Jan 9, 7:12 AM

Tue, Jan 8

probinson added a comment to D54769: [FileCheck] New option -warn.

Another problem would be if lit substitutions make FileCheck commands or their prefixes unrecognizable. I haven't looked for that usage yet.

Tue, Jan 8, 3:56 PM
probinson added a comment to D54465: [CodeGen] Fix bugs in LiveDebugVariables when debug labels are generated..

A few more comments.

Tue, Jan 8, 3:06 PM · debug-info
probinson added a comment to D55825: [FileCheck] Suppress old -v/-vv diags if dumping input.

Thanks for the ping. The code change looks straightforward, but a handful of questions on the tests.

Tue, Jan 8, 2:38 PM
probinson committed rL350641: Rename DIFlagFixedEnum to DIFlagEnumClass. NFC.
Rename DIFlagFixedEnum to DIFlagEnumClass. NFC
Tue, Jan 8, 9:56 AM
probinson committed rC350641: Rename DIFlagFixedEnum to DIFlagEnumClass. NFC.
Rename DIFlagFixedEnum to DIFlagEnumClass. NFC
Tue, Jan 8, 9:56 AM
probinson committed rL350636: Don't emit DW_AT_enum_class unless it's actually an 'enum class'..
Don't emit DW_AT_enum_class unless it's actually an 'enum class'.
Tue, Jan 8, 8:32 AM
probinson committed rC350636: Don't emit DW_AT_enum_class unless it's actually an 'enum class'..
Don't emit DW_AT_enum_class unless it's actually an 'enum class'.
Tue, Jan 8, 8:32 AM
probinson closed D56393: [DebugInfo] Don't emit DW_AT_enum_class unless it's actually an 'enum class'..
Tue, Jan 8, 8:32 AM · debug-info

Mon, Jan 7

probinson created D56393: [DebugInfo] Don't emit DW_AT_enum_class unless it's actually an 'enum class'..
Mon, Jan 7, 8:14 AM · debug-info

Fri, Jan 4

probinson added a comment to D56297: [DWARFUnit] Don't assume basic types..

So it looks like I should modify the frontend to represent this as a DerivedType with a baseType=SubroutineType. Is that right?

Fri, Jan 4, 2:40 PM

Thu, Jan 3

probinson added a comment to D55940: Detect incorrect FileCheck variable CLI definition.

Changes to CommandLine.* should be tested in llvm/unittests/Support/CommandLineTest.cpp, as a rule. We don't want to rely on FileCheck tests to verify code in Support.

Thu, Jan 3, 12:18 PM

Dec 20 2018

probinson added a comment to D49084: FileCheck: Add support for numeric variables and expressions.

I am willing to review this; however it is a very large patch, which will take time to go through. Also, after tomorrow, I am on holiday until 3 January.

Dec 20 2018, 12:48 PM
probinson added a comment to D55940: Detect incorrect FileCheck variable CLI definition.

This will permit -DVALUE= (with no value) is that a supported usage?
It also permits -D=10 which I daresay doesn't mean anything.

Dec 20 2018, 11:42 AM

Dec 18 2018

probinson added a comment to D55721: llvm-dwarfdump: Improve/fix pretty printing of array dimensions.

A couple small things. If Jonas is happy, I'm happy.

Dec 18 2018, 2:22 PM
probinson added a comment to D55281: debuginfo: Use symbol difference for CU length to simplify assembly reading/editing.

I'm just still not sure the extra cl::opt is worth it, though - are you? Is the extra coverage significant/important?

I'd rather see a test break locally (because it's target-agnostic and uses the flag) than wait for some NVPTX-target bot to break after I commit.

Do you trim the target set you build/dev with locally? Does that save much time - I build with the default, all the (non-experimental) targets enabled, so NVPTX failures do happen locally.

Dec 18 2018, 9:31 AM
probinson added a comment to D55281: debuginfo: Use symbol difference for CU length to simplify assembly reading/editing.

I'm just still not sure the extra cl::opt is worth it, though - are you? Is the extra coverage significant/important?

Dec 18 2018, 9:22 AM
probinson accepted D55712: [Driver][PS4] Do not implicitly link against asan or ubsan if -nostdlib or -nodefaultlibs on PS4..

I missed that filcab had said ok internally. Go ahead Pierre.

Dec 18 2018, 7:58 AM
probinson added a comment to D55281: debuginfo: Use symbol difference for CU length to simplify assembly reading/editing.

Still a bit curious about the other discussion about flags, etc.

Dec 18 2018, 6:46 AM

Dec 17 2018

probinson added a comment to D55281: debuginfo: Use symbol difference for CU length to simplify assembly reading/editing.

@aprantl @probinson - you folks haev any feelings about this? Looks like these flags are essentially unused (except for testing, which could equally be accomplished by tests that specify an NVPTX triple, I think (except on builds that aren't building an NVPTX backend)) & are a collection of features that are all only needed on NVPTX. I'd lean towards removing them all & gating this on NVPTX. Or on one flag rather than several fine-grained ones, if/when someone starts playing with a ptx assembler that can cope with these features.

But yeah, no big deal I guess - leads to a bit of confusion about who these flags are meant for, under what conditions some of these features might be enabled but not others, etc.

Dec 17 2018, 2:43 PM
probinson added a comment to D55712: [Driver][PS4] Do not implicitly link against asan or ubsan if -nostdlib or -nodefaultlibs on PS4..

LGTM but I will defer to @filcab as he is more familiar with the area.

Dec 17 2018, 9:22 AM
probinson accepted D55738: [FileCheck] Annotate input dump (final tweaks).

Much better, thanks! LGTM

Dec 17 2018, 8:16 AM
probinson accepted D53899: [FileCheck] Annotate input dump (7/7).

LGTM

Dec 17 2018, 8:16 AM
probinson added a comment to D55721: llvm-dwarfdump: Improve/fix pretty printing of array dimensions.

Note that the lower-bound might be implicit, but it's never "unknown."
We do need to account for the cases where the implicit lower-bound is 1. The Fortran people will need this, and the OpenVMS folks support several other languages that also have a default LB of 1.

Using [C] (C = count) for both the count-only and upper-bound-only cases suggests that we're being descriptive rather than carefully reflecting the attributes that are in the subrange DIE, which is fine. So, if an explicit LB matches the language default, then the display should also be [C] (where C = count if we have a count, or C = U + (1 - L) if we have upper_bound).

The only change you're suggesting is to detect when an explicit LB matches the language default, and treating that case the same as if the LB were not specified (in terms of how it's printed out)?

Dec 17 2018, 8:10 AM

Dec 14 2018

probinson added a comment to D55721: llvm-dwarfdump: Improve/fix pretty printing of array dimensions.

Note that the lower-bound might be implicit, but it's never "unknown."
We do need to account for the cases where the implicit lower-bound is 1. The Fortran people will need this, and the OpenVMS folks support several other languages that also have a default LB of 1.

Dec 14 2018, 3:02 PM
probinson added inline comments to D53899: [FileCheck] Annotate input dump (7/7).
Dec 14 2018, 2:06 PM
probinson added a comment to D53897: [FileCheck] Annotate input dump (5/7).

Yes, a follow-up would be easier. I'll just commit something. If you prefer a phab review first, that's absolutely fine for me, but let me know.

Dec 14 2018, 2:03 PM
probinson added inline comments to D53898: [FileCheck] Annotate input dump (6/7).
Dec 14 2018, 2:00 PM
probinson added a comment to D53899: [FileCheck] Annotate input dump (7/7).

Remove MatchTypeCount, which is unused as of this patch.

Dec 14 2018, 1:14 PM
probinson accepted D53898: [FileCheck] Annotate input dump (6/7).

LGTM

Dec 14 2018, 1:06 PM
probinson accepted D53897: [FileCheck] Annotate input dump (5/7).

The terminology around "possible" "expected" and "final" matches probably warrants a few lines of commentary somewhere, unless I missed it in an earlier patch. There are getting to be enough cases that the casual reader (ahem) can't just Zen their way through them all.
Can be done as a follow-up if you prefer.
One comment nit and LGTM.

Dec 14 2018, 12:57 PM

Dec 12 2018

probinson added a comment to D54487: Implement llvm.commandline named metadata.

LGTM although I think John should have a chance to get in a last word.

Dec 12 2018, 2:34 PM
probinson added a comment to D36993: [llvm-dwarfdump] Print type names in DW_AT_type DIEs.

FWIW, I ended up adding test coverage for some of this in r348954 while improving the printing of arrays to include array dimensions - hand written assembly, so yeah, it's a bit less than ideal to maintain (mostly having to edit both the abbreviation and the definition - a solution to that would be handy (& having to lookup the numeric values of the enums (DW_AT/DW_TAGs))) but I think it's worth having.

Dec 12 2018, 12:05 PM
probinson added inline comments to D52999: [FileCheck] Annotate input dump (1/7).
Dec 12 2018, 11:50 AM
probinson accepted D53896: [FileCheck] Annotate input dump (4/7).

These get to be pretty mechanical after a while, don't they? I was wondering how hard it would be for someone else, next time somebody wants to add a new directive kind, but it seems like not really a big deal and that's a good thing.
Again a comment needs fixing but otherwise LGTM.
I'll try to get to the rest tomorrow.

Dec 12 2018, 11:00 AM
probinson accepted D53894: [FileCheck] Annotate input dump (3/7).

Comment issue, otherwise LGTM.

Dec 12 2018, 10:49 AM
probinson accepted D53893: [FileCheck] Annotate input dump (2/7).

One small comment fix needed, otherwise LGTM.

Dec 12 2018, 10:44 AM
probinson added a comment to D52999: [FileCheck] Annotate input dump (1/7).

A couple of nits I didn't notice until looking at the next patch.

Dec 12 2018, 10:23 AM

Dec 10 2018

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

Abandoning dead patch. This wound up being done a different way.

Dec 10 2018, 2:13 PM
probinson accepted D52999: [FileCheck] Annotate input dump (1/7).

I like having the extra explanations at the end of the tilde lines, that's a great idea.
One unnecessary include left, and LGTM.

Dec 10 2018, 1:34 PM
probinson added a comment to D54487: Implement llvm.commandline named metadata.

Does the verifier bail out after the first failure? If not, you can combine the failure tests all into one test.

Dec 10 2018, 1:02 PM
probinson added a comment to D55272: [DebugInfo] Remove now un-necessary logic from HoistThenElseCodeToIf.
In D55272#1323317, @vsk wrote:

LGTM. @CarlosAlbertoEnciso, any thoughts on this?

Dec 10 2018, 12:43 PM

Dec 6 2018

probinson added inline comments to D55243: [mir] Serialize DILocation inline when not possible to use a metadata reference.
Dec 6 2018, 2:33 PM · debug-info

Dec 4 2018

probinson added a comment to D55281: debuginfo: Use symbol difference for CU length to simplify assembly reading/editing.

Easy enough to accommodate NVPTX, if you use DWARF version to pick constant size versus label diff. I don't remember what it takes to work out what the target is at this point, but version is obviously easy.

Dec 4 2018, 1:57 PM

Dec 3 2018

probinson added a comment to D53898: [FileCheck] Annotate input dump (6/7).

You're using !~~~ to flag: (a) CHECK-NOT that has a match, (b) CHECK-NEXT that isn't in the right place, (c) a discarded DAG match.
The first two are errors (which would be reported regardless of annotations) but the third is not. So, I'm not sure why it should have the same kind of annotation.

Dec 3 2018, 1:46 PM
probinson added inline comments to D54465: [CodeGen] Fix bugs in LiveDebugVariables when debug labels are generated..
Dec 3 2018, 12:58 PM · debug-info
probinson added a comment to D54769: [FileCheck] New option -warn.

I'm not sure which tests would be "simple", but any test that generates the FileCheck check file dynamically would certainly not work with such a script.

I don't think there are any of those - at least I rather hope not.

Dec 3 2018, 11:38 AM
probinson added a comment to D55227: [DebugInfo] Don't drop dbg.value's of nullptr.

I have no answer for the address-space questions. I'd say ask on llvm-dev, as it's worth asking in a more visible forum.
Assuming we are able to say that nullptr is zero for address space zero, the patch seems fine (and modifying an existing test is perfectly fine).

Dec 3 2018, 11:01 AM
probinson added a comment to D54348: Permit multiple .file directives with -g.

It looks like this patch would fail to reject cases such as:

.file 1 "a.c"
.file 1 "b.c"

and if that's true, it's a problem.

Dec 3 2018, 10:54 AM · debug-info
probinson added a reviewer for D53832: Make instrprof-set-dir-mode test tolerant of group ID: mattd.

+ Matt Davis who wrote the test
Sorry for the delay; feel free to add a 'ping' reply every week or so when you get no action.
Seems right to me but I think Matt would be a better choice to review.

Dec 3 2018, 10:47 AM
probinson accepted D54860: [llvm-dwarfdump] - Dump the older versions of .eh_frame/.debug_frame correctly..

The indentation in the test file looks a little funny; please make sure there are no hard tabs in the file. Aside from that LGTM.

Dec 3 2018, 9:49 AM

Nov 30 2018

probinson added a comment to D55091: Add --analyze option to llvm-dwarfdump.
In D55091#1314919, @vsk wrote:

Your DWARF analysis tool presumably needs to support diffing and output serialization. So would a comprehensive code size analysis tool. There's no reason why these two tools should have different diffing / serialization options. The latter shouldn't live within a debug info specific tool. However, there's no fundamental issue with adding DWARF to the long list of formats llvm-objdump already understands. Technically, it's just a matter of linking in llvm's DWARF libraries.

Nov 30 2018, 2:05 PM
probinson accepted D55137: Honor -fdebug-prefix-map when creating function names for the debug info..

LGTM aside from a grump about the test, which is totally up to you.

Nov 30 2018, 1:48 PM · debug-info
probinson added inline comments to D52999: [FileCheck] Annotate input dump (1/7).
Nov 30 2018, 1:37 PM
probinson added inline comments to D55137: Honor -fdebug-prefix-map when creating function names for the debug info..
Nov 30 2018, 12:55 PM · debug-info
probinson added inline comments to D52999: [FileCheck] Annotate input dump (1/7).
Nov 30 2018, 12:44 PM
probinson added a comment to D52999: [FileCheck] Annotate input dump (1/7).

Apologies for the delay. I haven't been ignoring this series; I was having internal qualms about the amount of effort to produce extensive annotations, and the value they might provide. But I've come down in favor of doing it.

Nov 30 2018, 11:56 AM
probinson added inline comments to D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries.
Nov 30 2018, 10:58 AM · debug-info
probinson added a comment to D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries.

When building the FileCheck binary with debug info, this patch makes the build artifacts ~1kb smaller.

Nov 30 2018, 10:09 AM · debug-info
probinson added a comment to D54769: [FileCheck] New option -warn.

But I still disagree with this one:

And if the input was this:

RUN: llc -mtriple=arm-linux-gnueabi %s -o - | FileCheck %s -check-prefix=FOO
define void @FOO() {
entry:
; FOO-LABEL: FOO:
; DOO:             mov  pc, lr
; FOO:             ret void
}

there is no reasonable way to flag the DOO line as a mistake. In fact, typos in the prefix are a tactic I've often used to "comment out" directives, for one reason or another, and I would not want that tactic to start triggering warnings.

I think this is the prime example where I would like to see and get a diagnostic. In this example, I think there are 2 bugs: 1 in the test, and 1 in FileCheck. There are probably many ways to temporarily comment out things and this is a "dangerous" one. Because allowing this relaxed FileCheck behaviour it's too easy to shout ourselves in the foot, and that's why we have so many test issues. The fact that we have so many test issues, shows that code review isn't always catching this. But I don't think code review should not really have to deal with this. We have a tool for this: FileCheck should do its checking, and do it properly. Besides 'commenting out' checks in this way, the other cause for this if you first had --check-prefixes=FOO,DOO, then some test refactoring happens, DOO is dropped from the --check-prefixes line, and the test becomes half useless, half correct.

Nov 30 2018, 9:27 AM
probinson added a comment to D54755: [DebugInfo] IR/Bitcode changes for DISubprogram flags.

While I was updating some internal testcases I wondered: why is DIFlagPrototype not a DISPFlag? Does this flag make sense on something other than functions?

Because I was not interested in moving existing flags from the old word to the new word in the first round. There are others that probably apply only to subprograms:
NoReturn, MainSubprogram; probably Thunk, All CallsDescribed; maybe Trivial, Explicit.
Really we should do some kind of audit, and do a bulk move in one go so we can minimize the bitcode-upgrade pain.

One additional problem is that the existing flags are exposed as part of the public API, i.e. to front-ends, so we'll need to work out how to avoid Bad Stuff(tm) if we repurpose any existing flag bits.

Another option here is not to merge them at all in bitcode (I didn't consider this before). You can add an abbreviation for a fixed-width field of 1 bit each:
https://llvm.org/docs/BitCodeFormat.html#fixed-width-value

Nov 30 2018, 8:55 AM · debug-info
probinson added a comment to D55113: [llvm-dwarfdump] - Stop printing the bogus empty section name on invalid dwarf..

Are those hard tabs in the test .s file? many editors do that automatically in assembler source, but the LLVM project doesn't want hard tabs.

Nov 30 2018, 7:50 AM

Nov 29 2018

probinson committed rL347918: Comment tweak requested in code review. NFC.
Comment tweak requested in code review. NFC
Nov 29 2018, 1:16 PM
probinson added a comment to D54755: [DebugInfo] IR/Bitcode changes for DISubprogram flags.

While I was updating some internal testcases I wondered: why is DIFlagPrototype not a DISPFlag? Does this flag make sense on something other than functions?

Because I was not interested in moving existing flags from the old word to the new word in the first round. There are others that probably apply only to subprograms:
NoReturn, MainSubprogram; probably Thunk, All CallsDescribed; maybe Trivial, Explicit.
Really we should do some kind of audit, and do a bulk move in one go so we can minimize the bitcode-upgrade pain.

Nov 29 2018, 10:34 AM · debug-info
probinson added a comment to D54755: [DebugInfo] IR/Bitcode changes for DISubprogram flags.

Nice!

Nov 29 2018, 10:23 AM · debug-info
probinson added a comment to D54755: [DebugInfo] IR/Bitcode changes for DISubprogram flags.

While I was updating some internal testcases I wondered: why is DIFlagPrototype not a DISPFlag? Does this flag make sense on something other than functions?

Nov 29 2018, 10:19 AM · debug-info

Nov 28 2018

probinson added a comment to D54043: Adding debug info to support Fortran (part 1).

My patches have landed, and look like they'll stick; please rebase and use the DISubprogram specific flags for the new Fortran flags.

Nov 28 2018, 5:10 PM · debug-info
probinson committed rC347807: [DebugInfo] NFC Clang test changes for: IR/Bitcode changes for DISubprogram….
[DebugInfo] NFC Clang test changes for: IR/Bitcode changes for DISubprogram…
Nov 28 2018, 1:21 PM
probinson committed rL347807: [DebugInfo] NFC Clang test changes for: IR/Bitcode changes for DISubprogram….
[DebugInfo] NFC Clang test changes for: IR/Bitcode changes for DISubprogram…
Nov 28 2018, 1:21 PM
probinson closed D54756: [DebugInfo] NFC Clang test changes for DISubprogram flags.
Nov 28 2018, 1:21 PM · debug-info
probinson committed rL347806: [DebugInfo] IR/Bitcode changes for DISubprogram flags..
[DebugInfo] IR/Bitcode changes for DISubprogram flags.
Nov 28 2018, 1:18 PM
probinson closed D54755: [DebugInfo] IR/Bitcode changes for DISubprogram flags.
Nov 28 2018, 1:18 PM · debug-info
probinson added a comment to D54756: [DebugInfo] NFC Clang test changes for DISubprogram flags.

Everybody with out-of-tree tests will be excited ;-)

Nov 28 2018, 11:05 AM · debug-info
probinson added a comment to D54755: [DebugInfo] IR/Bitcode changes for DISubprogram flags.

I think it would be nice to have one small explicit "DISubprogram version 4" .bc upgrade test, otherwise this LGTM now. Thanks!

Nov 28 2018, 9:20 AM · debug-info
probinson added a comment to D54756: [DebugInfo] NFC Clang test changes for DISubprogram flags.

Ping. @aprantl approved the dependent patch D54755.

Nov 28 2018, 9:16 AM · debug-info

Nov 27 2018

probinson added a comment to D54327: Adding debug info to support Fortran (part 3).

I am curious how debug info for common blocks plays with LTO. I think we would not want different common-block descriptions to be de-duplicated just because the global name was the same.

Nov 27 2018, 2:51 PM · debug-info
probinson added a comment to D54043: Adding debug info to support Fortran (part 1).

Thanks, Paul.

Or this could go in first and supply additional tests for your new bit format. :)

For what it's worth, the overloaded bit-encoding follows the precedent already established by LLVMDIFlagIndirectVirtualBase.

Nov 27 2018, 2:32 PM · debug-info
probinson updated the diff for D54755: [DebugInfo] IR/Bitcode changes for DISubprogram flags.

Don't emit unused fields to the bitcode. Accommodate the new variant in the reader.

Nov 27 2018, 2:24 PM · debug-info
probinson added a comment to D54043: Adding debug info to support Fortran (part 1).

I believe all the comments have been addressed. Is there anything else required on our end?

Nov 27 2018, 2:11 PM · debug-info

Nov 26 2018

probinson added a comment to D54769: [FileCheck] New option -warn.

And I do remember a possibly valid use case for emitting a diagnostic. I've seen quite a few tests with names like test-pr12345.ll, that were reproducers of compiler crashes, but only contained CHECK-LABELS. I called them dubious tests before (but we don't want to use that term), because these tests are happy when a functions compiles, but don't tests anything whereas they probably should. There are probably a few exceptions where only CHECK-LABELs are okay, so I think a warning along the lines of "you're only checking labels, is that what you really want?" should be fair?

Nov 26 2018, 5:30 PM
probinson added a comment to D54769: [FileCheck] New option -warn.

So, I'm not convinced that we can have a diagnostic for your case 3 that would really be meaningful and useful.

That is really unfortunate and problematic. Apologies, but just for completeness, this was that example:

RUN: llc -mtriple=arm-linux-gnueabi %s -o - | FileCheck %s -check-prefix=FOO
define void @FOO() {
entry:
  ; FOO-LABEL: FOO:
  ; DOO:             mov  pc, lr
  ret void
}

It's really not difficult to make this mistake, and I really would like to solve this one too. My previous approach might be wrong, so I am going to look again if I can get it to reliably error in this case.

Nov 26 2018, 4:43 PM
probinson added a comment to D54769: [FileCheck] New option -warn.

I am unsure about 3), i.e. I can't remember/am unsure if we can avoid false positives. I will look into this again, or if someone can remind me that's fine too :-). But I thought this was the motivating case for emitting a diagnostic: if a false positive cannot be easily be avoided, an opt-in warning is the best we can do at the moment. The use case would indeed be to enable this while developing new tests.

Nov 26 2018, 4:03 PM
probinson added a comment to D54769: [FileCheck] New option -warn.

You/others could checkin fixes as they find them rather than accumulating one large patch - then when it's close to done, you send out the patch to add the error to avoid regressions?

Adding -warn now makes it easier for anyone to contribute to this cleanup and doesn't hurt anyone who doesn't care about it.

Nov 26 2018, 2:47 PM
probinson added a comment to D54755: [DebugInfo] IR/Bitcode changes for DISubprogram flags.

I'm assuming we already have plenty of DISubprograms with flags in the old format as .bc tests that cover the bitcode upgrade?

Nov 26 2018, 1:09 PM · debug-info
probinson added a comment to D54755: [DebugInfo] IR/Bitcode changes for DISubprogram flags.

Ping.

Nov 26 2018, 7:54 AM · debug-info

Nov 21 2018

probinson added a comment to D54769: [FileCheck] New option -warn.

This is small enough that I'd prefer to see it incorporated into the patch with the "real" warning, rather than have this temporary placeholder diagnostic.

Nov 21 2018, 8:29 AM

Nov 20 2018

probinson created D54756: [DebugInfo] NFC Clang test changes for DISubprogram flags.
Nov 20 2018, 8:02 AM · debug-info
probinson created D54755: [DebugInfo] IR/Bitcode changes for DISubprogram flags.
Nov 20 2018, 8:00 AM · debug-info

Nov 19 2018

probinson committed rL347271: It's its.
It's its
Nov 19 2018, 2:56 PM
probinson committed rL347246: Fix build break from r347239.
Fix build break from r347239
Nov 19 2018, 10:55 AM
probinson committed rC347239: [DebugInfo] DISubprogram flags get their own flags word. NFC..
[DebugInfo] DISubprogram flags get their own flags word. NFC.
Nov 19 2018, 10:32 AM
probinson committed rL347239: [DebugInfo] DISubprogram flags get their own flags word. NFC..
[DebugInfo] DISubprogram flags get their own flags word. NFC.
Nov 19 2018, 10:32 AM