Page MenuHomePhabricator

probinson (Paul Robinson)
User

Projects

User Details

User Since
May 9 2013, 11:10 AM (402 w, 4 d)

Recent Activity

Today

probinson added a comment to D82363: [DebugInfo] Add new instruction and expression operator for variadic debug values.

W.r.t the replacement of DBG_VALUE with DBG_VALUE_LIST, the current plan is to make that all happen in a patch following this stack. The patch itself should mostly be deletion of old code and simplifying where possible, renaming DBG_VALUE_LIST to DBG_VALUE, adding an extra operand to the new DBG_VALUE to represent directness, adding several tests, and updating existing tests as appropriate.

Mon, Jan 25, 9:04 AM · Restricted Project, debug-info

Fri, Jan 22

probinson committed rG25fefa5a098e: [RGT][TextAPI] Remove a zero-trip loop and the assertions within it (authored by probinson).
[RGT][TextAPI] Remove a zero-trip loop and the assertions within it
Fri, Jan 22, 3:08 PM
probinson closed D95259: [RGT][TextAPI] Remove a zero-trip loop and the assertions within it.
Fri, Jan 22, 3:08 PM · Restricted Project
probinson committed rG6ea7ecbb72aa: [RGT] Don't use EXPECT* macros in a subprocess that exits by signalling (authored by probinson).
[RGT] Don't use EXPECT* macros in a subprocess that exits by signalling
Fri, Jan 22, 3:05 PM
probinson closed D95256: [RGT] Don't use EXPECT* macros in a test subprcess that exits by signalling..
Fri, Jan 22, 3:05 PM · Restricted Project
probinson committed rG6ef95056b9dc: [RGT][ADT] Remove test assertion that will not be executed (authored by probinson).
[RGT][ADT] Remove test assertion that will not be executed
Fri, Jan 22, 3:04 PM
probinson closed D95255: [RGT][ADT] Remove test assertion that will not be executed..
Fri, Jan 22, 3:04 PM · Restricted Project
probinson added inline comments to D95257: [RGT][GlobalISel] Add missing setUp() calls to legalizer unittests.
Fri, Jan 22, 1:14 PM · Restricted Project
probinson requested review of D95259: [RGT][TextAPI] Remove a zero-trip loop and the assertions within it.
Fri, Jan 22, 12:17 PM · Restricted Project
probinson added a comment to D95258: [RGT][ProfileData] Correct a test assertion.

It's possible that the InstantiationGroups result is not supposed to be empty; in which case, something deeper is wrong, and I can abandon this patch and file a bug. Let me know.

Fri, Jan 22, 12:14 PM · Restricted Project
probinson requested review of D95258: [RGT][ProfileData] Correct a test assertion.
Fri, Jan 22, 12:13 PM · Restricted Project
probinson requested review of D95257: [RGT][GlobalISel] Add missing setUp() calls to legalizer unittests.
Fri, Jan 22, 12:06 PM · Restricted Project
probinson requested review of D95256: [RGT] Don't use EXPECT* macros in a test subprcess that exits by signalling..
Fri, Jan 22, 11:59 AM · Restricted Project
probinson requested review of D95255: [RGT][ADT] Remove test assertion that will not be executed..
Fri, Jan 22, 11:56 AM · Restricted Project

Thu, Jan 21

probinson added inline comments to D95158: Use DataExtractor to decode SLEB128 in android_relas..
Thu, Jan 21, 7:05 PM · Restricted Project
probinson added a comment to D95114: HowToReleaseLLVM: Add annual release schedule template.

I'd expect the pace of preparing the release to be basically the same regardless of start date, so the milestones would make more sense as start date plus N weeks.
If you want wiggle room for the start date, that's fine, anything that expresses "late January" and "late July" will work. I thought it was going to be more definitive, what with specific week numbers and all.

Thu, Jan 21, 8:09 AM · Restricted Project
probinson added a comment to D95114: HowToReleaseLLVM: Add annual release schedule template.

I think "Week number" is too ambiguous to be a guide. If January starts on the last day of the week, does that still count as week#1? What day does the week start on, anyway--much of the world starts the week on Sunday, much of the world starts the week on Monday.
"Fourth Tuesday in January/July" is unambiguous and makes everything easier to plan. Using "Start + N weeks" for the rest of the target dates is fine.

Thu, Jan 21, 6:25 AM · Restricted Project

Tue, Jan 19

probinson added a comment to D94882: [MC] Upgrade DWARF version to 5 upon .file 0.

Catching up on this, sorry...

Tue, Jan 19, 8:54 AM · Restricted Project

Mon, Jan 11

probinson committed rG1f9c29228cec: [FastISel] NFC: Clean up unnecessary bookkeeping (authored by probinson).
[FastISel] NFC: Clean up unnecessary bookkeeping
Mon, Jan 11, 9:41 AM
probinson committed rGbe179b9946f6: [FastISel] NFC: Remove obsolete -fast-isel-sink-local-values option (authored by probinson).
[FastISel] NFC: Remove obsolete -fast-isel-sink-local-values option
Mon, Jan 11, 9:33 AM
probinson committed rGc161775decdd: [FastISel] Flush local value map on every instruction (authored by probinson).
[FastISel] Flush local value map on every instruction
Mon, Jan 11, 8:33 AM
probinson closed D91734: [FastISel] Flush local value map on every instruction.
Mon, Jan 11, 8:33 AM · Restricted Project, Restricted Project, debug-info
probinson committed rGe5eb5c8a7f30: NFC: Use -LABEL more (authored by probinson).
NFC: Use -LABEL more
Mon, Jan 11, 8:24 AM

Wed, Jan 6

probinson added a comment to D91734: [FastISel] Flush local value map on every instruction.

But, yes, we could possibly do better with more strategic is_stmt, but that's a big/complex piece of work to tackle.

Wed, Jan 6, 8:58 AM · Restricted Project, Restricted Project, debug-info
probinson added a comment to D91734: [FastISel] Flush local value map on every instruction.

I haven't looked closely, but I take it this one test modification seems reasonable to you? I guess we're stepping into some subexpressions on a function call that we previously didn't? (they didn't have any instructions attributed to them until now, but it's not unreasonable that they have them attributed - for materializing constants to pass to a function call when the function call/constants are split over multiple lines, etc)

Wed, Jan 6, 8:19 AM · Restricted Project, Restricted Project, debug-info

Tue, Jan 5

probinson added a comment to D91734: [FastISel] Flush local value map on every instruction.

This version of the patch avoids the weirdness I was seeing with prolog instructions in certain cases.

Tue, Jan 5, 1:40 PM · Restricted Project, Restricted Project, debug-info
probinson updated the diff for D91734: [FastISel] Flush local value map on every instruction.

Change how PHIs are handled; if the operand has a debug location, use it, otherwise don't set a debug location. Then, flushLocalValueMap() will look at the first local-value instruction; if it doesn't already have a debug location, we copy the location from the first instruction after the local-value instructions (which should have the debug location of the original IR instruction).

Tue, Jan 5, 1:33 PM · Restricted Project, Restricted Project, debug-info

Dec 16 2020

probinson added a comment to D83892: [clang][cli] Port CodeGen option flags to new option parsing system.

Thanks Duncan! I (or someone) will play around with that and see what we need to do. May take a little while, as we're about to freeze a release and then go on break for two weeks, but good to know there's a straightforward path.

Dec 16 2020, 12:37 PM · Restricted Project, Restricted Project
probinson added a comment to D83892: [clang][cli] Port CodeGen option flags to new option parsing system.

One thing this patch does, is make decisions about default behavior static. Meaning, the option behavior cannot depend on other options; specifically, it can't be based on the triple, which allows target-specific customization. PS4 certainly has cases where our defaults are different from the usual ones, and I'd kind of think that was true for other targets as well.

Dec 16 2020, 8:03 AM · Restricted Project, Restricted Project

Dec 14 2020

probinson added a comment to D91734: [FastISel] Flush local value map on every instruction.

I've run this through our copy of the GDB suite (8.3, not sure how old that is). There are 10 differences, with and without the patch.

Dec 14 2020, 2:48 PM · Restricted Project, Restricted Project, debug-info

Dec 10 2020

probinson requested review of D91734: [FastISel] Flush local value map on every instruction.

This is still showing as approved; I'm going to try "Request Review" to see if that helps.

Dec 10 2020, 12:05 PM · Restricted Project, Restricted Project, debug-info
probinson updated the diff for D91734: [FastISel] Flush local value map on every instruction.

Rebase and combine cf1cc774d and dc35368c, plus revise handling constants materialized due to PHIs.
I plan to do some build-time and object size measurements, similar to the first time around.
I plan to run this against the gdb test suite, or at least the local copy we have here at Sony, and post my findings.

Dec 10 2020, 9:29 AM · Restricted Project, Restricted Project, debug-info
probinson reopened D91734: [FastISel] Flush local value map on every instruction.

Reopening, will upload a new diff in a moment that modifies how PHIs are handled. I intend to run the gdb suite on this, will post findings when I have them.

Dec 10 2020, 8:27 AM · Restricted Project, Restricted Project, debug-info

Dec 9 2020

probinson added a comment to D91734: [FastISel] Flush local value map on every instruction.

Yeah, it boils down to something like this:

void f1(const char*, const char*) { }
int main() {
  char prog[1];

  f1(prog,
     prog);
}

Without the patch, you step to the 'f1' line, and then step into or over the function call (step or next).
But with these patches applied - you step to the 'f1(' line, then the 'prog);' line, then back to the 'f1(' line, then into/over the function call.

Again, could be acceptable - if those arguments had specific non-trivial code in them (like other function calls) you'd certainly get that step forward/backward behavior - but it's notewhorthy that this is change would make more cases like that & it'd be good to understand why/if that's a good thing overall.

Dec 9 2020, 11:08 AM · Restricted Project, Restricted Project, debug-info

Dec 8 2020

probinson added a comment to D91734: [FastISel] Flush local value map on every instruction.

And with these changes together, breaking on the function breaks on line 5 (presumably because this is creating the constant used by the second '&&' which is on line 5) instead of line 3. Not the worst thing, but I imagine given Sony's interest in less "jumpy" line tables, this might not be super desirable.

It's breaking on an xorl %eax,%eax which is produced by the PHI at the end of the conditional expression, which now has the source location of the operator at the top of the expression tree, which is the second && operator, yeah. Not the best. (FTR, the jumpiness complaints we get are usually more egregious, hopping between different source statements somewhat arbitrarily; not sure anyone would complain too loudly about hopping around within an expression. We see less of that in any case, because we suppress column info, but still.)

Dec 8 2020, 2:25 PM · Restricted Project, Restricted Project, debug-info

Dec 3 2020

probinson added inline comments to D92606: Clean up debug locations for logical-and/or expressions.
Dec 3 2020, 2:37 PM · Restricted Project, debug-info
probinson added a comment to D91734: [FastISel] Flush local value map on every instruction.

See D92606 for a front-end patch to improve locations in the IR.
That, plus reapplying this patch, should help out GDB. I haven't had a chance to run the suite myself with both patches applied and I'm basically off tomorrow, so if @dblaikie doesn't have a chance, I'll do that on Monday.

Dec 3 2020, 1:59 PM · Restricted Project, Restricted Project, debug-info
probinson requested review of D92606: Clean up debug locations for logical-and/or expressions.
Dec 3 2020, 1:56 PM · Restricted Project, debug-info
probinson added a comment to D91734: [FastISel] Flush local value map on every instruction.

Sometihng like this seems plausible to me:

Dec 3 2020, 7:59 AM · Restricted Project, Restricted Project, debug-info

Dec 2 2020

probinson added a comment to D91734: [FastISel] Flush local value map on every instruction.

Just for grins, change the && to || and see what happens...
The xorl becomes a movb $1 (no surprise there). But, that instruction no longer has line-0, instead it becomes part of the prologue.
This tells me that the xorl had an explicit line 0, while the 'movb $1' has no location (a subtle but important difference).

Dec 2 2020, 12:01 PM · Restricted Project, Restricted Project, debug-info
probinson added a comment to D91734: [FastISel] Flush local value map on every instruction.

@rnk is correct that the phi's source location is propagating to the xorl. I modified the .ll file to put a different line number on the phi, and the xorl picked it up. This is clearly on the front-end to get right.

Dec 2 2020, 10:25 AM · Restricted Project, Restricted Project, debug-info
probinson added a comment to D91734: [FastISel] Flush local value map on every instruction.

The constant materialization (xorl) obviously does not belong in the prologue; I think the instruction ordering with the patch is preferable.
But, clearly we'd rather not have it attributed to line 0, but the line it actually belongs to. I'll start looking into this. Offhand the reduced example appears to be not unreasonably large :-)

Dec 2 2020, 7:46 AM · Restricted Project, Restricted Project, debug-info

Dec 1 2020

probinson added a comment to D92204: # Enter a commit message. commit message # # Changes: # llvm/CMakeLists.txt mlir/lib/Pass/Pass.cpp.

The patch deletes llvm/CMakeLists.txt, which is surely not something you want to do.

Dec 1 2020, 12:45 PM · Restricted Project, Restricted Project

Nov 30 2020

probinson committed rG3fd39d3694d3: [FastISel] NFC: Clean up unnecessary bookkeeping (authored by probinson).
[FastISel] NFC: Clean up unnecessary bookkeeping
Nov 30 2020, 12:29 PM
probinson closed D92338: [FastISel] NFC: Clean up unnecessary bookkeeping.
Nov 30 2020, 12:29 PM · Restricted Project
probinson added a comment to D92338: [FastISel] NFC: Clean up unnecessary bookkeeping.

The tests all pass, but I wasn't *quite* daring enough to commit this without a second opinion.

Nov 30 2020, 11:48 AM · Restricted Project
probinson requested review of D92338: [FastISel] NFC: Clean up unnecessary bookkeeping.
Nov 30 2020, 11:48 AM · Restricted Project
probinson committed rGa474657e30ed: [FastISel] NFC: Remove obsolete -fast-isel-sink-local-values option (authored by probinson).
[FastISel] NFC: Remove obsolete -fast-isel-sink-local-values option
Nov 30 2020, 10:56 AM

Nov 25 2020

probinson abandoned D90776: [FastISel] Sink more materializations to first use.

Abandoned in favor of D91734.

Nov 25 2020, 11:40 AM · debug-info, Restricted Project
probinson committed rGdc35368ccf17: Remove static function unused after cf1c774. (authored by probinson).
Remove static function unused after cf1c774.
Nov 25 2020, 10:44 AM
probinson committed rGcf1c774d6ace: [FastISel] Flush local value map on ever instruction (authored by probinson).
[FastISel] Flush local value map on ever instruction
Nov 25 2020, 10:05 AM
probinson closed D91734: [FastISel] Flush local value map on every instruction.
Nov 25 2020, 10:05 AM · Restricted Project, Restricted Project, debug-info

Nov 20 2020

probinson added a comment to D91761: [FileCheck] Add check modifier capability.

I have to say, I'm not super enthused about this feature. Typically we've added directives when there's no other way to achieve the desired effect

SAME could be achieved without needing SAME, COUNT too, and NEXT could be folded in too by extending to multi-line regex and verifying no newlines so too DAG. So I believe there is a goal with directives to make checks readable for users & simple to interpret rather than just possible. Escaping is possible but obscures what is being tested and easy to get wrong.

Nov 20 2020, 7:04 AM · Restricted Project

Nov 19 2020

probinson added a comment to D91761: [FileCheck] Add check modifier capability.

I have to say, I'm not super enthused about this feature. Typically we've added directives when there's no other way to achieve the desired effect; the LITERAL feature doesn't meet that bar. It's a way to simplify something that you could do already. Not to say I'm going to insist on killing it, but I would like to hear more justification.

Nov 19 2020, 5:38 PM · Restricted Project
probinson added a comment to D91043: [DebugInfo] Fix convert-loclist.ll.

The fix seems fine to me - but I think there's something that at least I'm not understanding and would like to understand, since it's related to understanding the correctness of this fix: Why are there a bunch of other uses of %llc_dwarf in the DebugInfo/X86 tests - why don't they have the same problem as this test?

Nov 19 2020, 11:38 AM · Restricted Project
probinson added a comment to D91734: [FastISel] Flush local value map on every instruction.

I don't reproduce the HWASan failure on my Linux.

Nov 19 2020, 10:52 AM · Restricted Project, Restricted Project, debug-info
probinson updated the diff for D91734: [FastISel] Flush local value map on every instruction.

Fix an lld test and two lldb tests.

Nov 19 2020, 10:50 AM · Restricted Project, Restricted Project, debug-info
probinson added a comment to D91734: [FastISel] Flush local value map on every instruction.
In D91734#2403719, @rnk wrote:

Thanks! It looks like the presubmit/Harbormaster testing found some more test failures that need addressing.

Nov 19 2020, 8:25 AM · Restricted Project, Restricted Project, debug-info
probinson added a comment to D91734: [FastISel] Flush local value map on every instruction.

FTR, I plan to remove the -fast-isel-sink-local-instructions option. I think there might be some now-unused fields as well, such as LastFlushPoint, that could be removed. These are NFC follow-ups that can be deferred, this patch is big enough as it is.

Nov 19 2020, 7:55 AM · Restricted Project, Restricted Project, debug-info

Nov 18 2020

probinson added inline comments to D91734: [FastISel] Flush local value map on every instruction.
Nov 18 2020, 1:30 PM · Restricted Project, Restricted Project, debug-info
probinson updated the diff for D91734: [FastISel] Flush local value map on every instruction.

Some sort of pilot error on that first diff. This should be better.

Nov 18 2020, 1:30 PM · Restricted Project, Restricted Project, debug-info
probinson added a comment to D91734: [FastISel] Flush local value map on every instruction.

Dammit. Visual Studio keeps running clang-format on me. I thought I had that turned off and fixed up the formatting but apparently not. I'll make sure those bits aren't in the next round.

Nov 18 2020, 11:58 AM · Restricted Project, Restricted Project, debug-info
probinson added a comment to D90776: [FastISel] Sink more materializations to first use.

See D91734 for an alternate approach as suggested by @rnk .

Nov 18 2020, 11:46 AM · debug-info, Restricted Project
probinson requested review of D91734: [FastISel] Flush local value map on every instruction.
Nov 18 2020, 11:45 AM · Restricted Project, Restricted Project, debug-info

Nov 10 2020

probinson added a comment to D91191: WIP: [FileCheck] Add undefined prefix report tool.

Thanks, Joel! It will likely take a few weeks for me to finish up the unittests part of my project, at which point this seems like a nice starting point for the FileCheck part. The basic infrastructure is definitely there.

Nov 10 2020, 2:41 PM · Restricted Project
probinson added a comment to D90776: [FastISel] Sink more materializations to first use.

Building Clang with Clang, adding the prototype patch improved build time by a barely perceptible 0.5%.

Nov 10 2020, 1:14 PM · debug-info, Restricted Project
probinson added a comment to D90776: [FastISel] Sink more materializations to first use.

I prototyped a patch that would

  • call flushLocalValueMap() at the top of selectInstruction()
  • turn off the instruction-sinking stuff
  • not erase the DbgLoc applied to local value instructions
Nov 10 2020, 9:53 AM · debug-info, Restricted Project
probinson added a reverting change for rGe7256825d573: The arm64 triple requires AArch64 not ARM target: rGdef26af4eab0: Revert "The arm64 triple requires AArch64 not ARM target".
Nov 10 2020, 8:27 AM
probinson committed rGdef26af4eab0: Revert "The arm64 triple requires AArch64 not ARM target" (authored by probinson).
Revert "The arm64 triple requires AArch64 not ARM target"
Nov 10 2020, 8:27 AM
probinson committed rGe7256825d573: The arm64 triple requires AArch64 not ARM target (authored by probinson).
The arm64 triple requires AArch64 not ARM target
Nov 10 2020, 8:19 AM

Nov 9 2020

probinson committed rG920befb337ae: [FastISel] Reduce spills around mem-intrinsic calls (authored by probinson).
[FastISel] Reduce spills around mem-intrinsic calls
Nov 9 2020, 9:50 AM
probinson closed D90877: [FastISel] Reduce spills around mem-intrinsic calls.
Nov 9 2020, 9:50 AM · debug-info, Restricted Project

Nov 5 2020

probinson added a comment to D90776: [FastISel] Sink more materializations to first use.

I think we should also consider scrapping the local value map or maybe flushing it after selecting each instruction. I don't think we're getting a whole lot of value from sharing local values between multiple non-call instructions.

Nov 5 2020, 6:47 PM · debug-info, Restricted Project
probinson requested review of D90877: [FastISel] Reduce spills around mem-intrinsic calls.
Nov 5 2020, 1:13 PM · debug-info, Restricted Project
probinson added reviewers for D90776: [FastISel] Sink more materializations to first use: MatzeB, qcolombet.

+ people who reviewed D43093.

Nov 5 2020, 10:42 AM · debug-info, Restricted Project

Nov 4 2020

probinson requested review of D90776: [FastISel] Sink more materializations to first use.
Nov 4 2020, 10:38 AM · debug-info, Restricted Project

Oct 21 2020

probinson added a comment to D89838: [DebugInfo] Fix legacy ZExt emission when FromBits >= 64 (PR47927).

Ummm the default expression-stack type is address-sized, and I'm not aware that we support machines with address sizes > 64 bits?
(DWARF 5 does have a typed stack, but IIUC this path is not used for DWARF 5.)

Oct 21 2020, 7:26 AM · Restricted Project

Sep 30 2020

probinson updated subscribers of D83069: [LIT] error if directly named test won't be run indirectly.

Adding @jdenny and @thopre who I think have been messing with lit recently.

Sep 30 2020, 7:24 AM · Restricted Project

Sep 29 2020

probinson added a comment to D78778: [AMDGPU] Add SupportsDebugUnwindInformation to MCAsmInfo.

I was not aware that .eh_frame needed to be an ALLOC section, although it makes sense now that you say it; thanks for that!

Sep 29 2020, 12:53 PM · debug-info, Restricted Project
probinson added a comment to D78778: [AMDGPU] Add SupportsDebugUnwindInformation to MCAsmInfo.

I haven't touched the streamer side of AsmPrinter in a long time, but...
On PS4 we default to -fno-exceptions and while the section ends up named .eh_frame instead of .debug_frame the content is fine; we've had no complaints from our debugger folks. Aside from the introduction of the .cfi directives (which you do want), I'm not aware that there's any codegen consequence to this combination.
So, I'm not clear why a special streamer is really necessary.

Sep 29 2020, 7:57 AM · debug-info, Restricted Project
probinson added a comment to D87811: [CodeGen] emit CG profile for COFF object file.

Can we have a separate NFC refactor commit, to move that lump of code into its own emitCGProfile method? It might make the functional bits easier to follow. Also we have a private change in that area, and the ping-pong for this change has been a real headache.

Sep 29 2020, 7:16 AM · Restricted Project

Sep 23 2020

probinson added a comment to D68620: DebugInfo: Use base address selection entries for debug_loc.

TL;DR: It's all good.

Sep 23 2020, 8:55 AM · Restricted Project

Sep 17 2020

probinson added a comment to D68620: DebugInfo: Use base address selection entries for debug_loc.

(as an aside: do you/@probinson have much of a sense of how much binary size increase you'd trade for object size reductions in this space? If binary size is ulmtimately what you care most about, I'm guessing maybe debug_loc base address specifiers will never be a win for you & perhaps we should just group them under a flag, maybe refactor the debug_ranges base address specifier flag to cover both (the flag there was introduced due to a gold+gdb_index+32 bit binary bug, unfortunately, but lumping them together seems OK-ish to me))

I'm not sure, I'll defer to @probinson on that.

Sep 17 2020, 8:19 AM · Restricted Project
probinson added inline comments to D87656: [llvm-dwarfdump] --show-sources option to show all sources.
Sep 17 2020, 7:39 AM · Restricted Project

Sep 14 2020

probinson updated subscribers of D68620: DebugInfo: Use base address selection entries for debug_loc.

@probinson - do you have any state here? Whether this is an ongoing issue, whether the improvements to avoid unnecessary debug_loc have reduced the overhead sufficiently, etc?

Sep 14 2020, 8:09 AM · Restricted Project

Sep 1 2020

probinson accepted D86965: Do not emit "-tune-cpu generic" for PS4 platform.

Couple of nits and LGTM.

Sep 1 2020, 12:28 PM · Restricted Project

Aug 6 2020

probinson added a comment to D85239: [DOCS] Add more detail to stack protector documentation.

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.

Aug 6 2020, 10:51 AM · Restricted Project

Aug 4 2020

probinson added a comment to D85239: [DOCS] Add more detail to stack protector documentation.

Looks okay (one grammar nit), probably worth waiting for someone else to chime in.

Aug 4 2020, 12:46 PM · Restricted Project

Jul 30 2020

probinson added a project to D84802: Add control over debug lines dropping: debug-info.
Jul 30 2020, 10:46 AM · debug-info, Restricted Project
probinson added a comment to D84802: Add control over debug lines dropping.

No test?

Jul 30 2020, 10:46 AM · debug-info, Restricted Project

Jul 28 2020

probinson accepted D68061: [docs] Document pattern of using CHECK-SAME to skip irrelevant lines.

@rupprecht if you are still motivated to land this after so long, the only thing I see missing is to update the example from CHECK-SAME: 1{{$}} to CHECK-SAME: {{ 1$}} and then it LGTM. If not, there seems to be enough interest that someone else can land it?

Jul 28 2020, 7:38 AM · Restricted Project

Jul 23 2020

probinson accepted D83462: [DWARF] Avoid entry_values production for SCE.

LGTM

Jul 23 2020, 8:19 AM · Restricted Project, debug-info

Jul 20 2020

probinson added a comment to D84094: [DebugInfo] Fix misleading using of DWARF forms with DIELabel. NFCI..

It looks like you have been tweaking a number of related SizeOf() methods. Is it practical to have just one? In a parent class, or even as a free function (as the size computation depends only on form/format, and is not related to the class at all).

Jul 20 2020, 8:56 AM · debug-info, Restricted Project
probinson added a comment to D84106: [llvm-readobj] Construct relocation-aware DWARFDataExtractor to decode .eh_frame addresses correctly.

I agree with @grimar as it looks like the change is from a DWARFDataExtractor without relocations to one with relocations.
Regarding header order, in cases like this it's usual to do an NFC commit to sort the headers that are there, and then the functional patch just adds the new one in the right place.

Jul 20 2020, 8:45 AM · Restricted Project

Jul 15 2020

probinson added a comment to D83834: Add test utility 'split-file'.

Just wanted to say I agree this can be better than the Inputs/ idiom, especially for comparatively short inputs. We have some downstream tests that use a bunch of echo commands to generate a second file, but an extract utility would be cleaner IMO.

Jul 15 2020, 6:38 AM · Restricted Project

Jul 14 2020

probinson added a comment to D83468: [Debuginfo] Fix for PR46653.

According to https://bugs.llvm.org/show_bug.cgi?id=46653#c3 there is no problem on x86-64, suggesting the fix is more likely to be in the AArch64 target instead of in generic code. It's not uncommon for passes to lose source locations, or fail to add them when they should, so that is more likely to be the cause of the AArch64 issue.

Jul 14 2020, 12:28 PM · debug-info, Restricted Project
probinson added inline comments to D83468: [Debuginfo] Fix for PR46653.
Jul 14 2020, 12:25 PM · debug-info, Restricted Project
probinson added a comment to D83725: [llvm-mc] Add --doc-id=<id> to support multiple documents in a file.

Wouldn't using .ifdef directives with --defsym on the command line work equally well? At least for .s files run through llvm-mc.
There are examples of this in the lit tests already, easy enough to find.

Jul 14 2020, 6:13 AM · Restricted Project

Jul 9 2020

probinson accepted D83091: [FileCheck] Improve -dump-input documentation.

LGTM.

Jul 9 2020, 2:09 PM · Restricted Project
probinson added a comment to D83462: [DWARF] Avoid entry_values production for SCE.

So the tuning here for SCE is also a "does not support" or something else?

Jul 9 2020, 2:01 PM · Restricted Project, debug-info