Page MenuHomePhabricator

probinson (Paul Robinson)
User

Projects

User Details

User Since
May 9 2013, 11:10 AM (413 w, 2 d)

Recent Activity

Fri, Apr 9

probinson committed rG206343f319da: [RGT] Disable some tests on Windows at compile-time, not runtime (authored by probinson).
[RGT] Disable some tests on Windows at compile-time, not runtime
Fri, Apr 9, 10:06 AM
probinson committed rGb7578f9d5a46: [RGT] Tweak test so assertion is always executed (authored by probinson).
[RGT] Tweak test so assertion is always executed
Fri, Apr 9, 8:11 AM

Wed, Apr 7

probinson committed rG676a9ab5e406: Remove .gitignore entries not relevant in the monorepo. (authored by probinson).
Remove .gitignore entries not relevant in the monorepo.
Wed, Apr 7, 12:26 PM
probinson closed D100049: Remove .gitignore entries not relevant in the monorepo..
Wed, Apr 7, 12:25 PM · Restricted Project, Restricted Project
probinson requested review of D100049: Remove .gitignore entries not relevant in the monorepo..
Wed, Apr 7, 10:27 AM · Restricted Project, Restricted Project

Tue, Apr 6

probinson committed rG04b3c8c52c54: Pass -fcrash-diagnostics-dir along to LLVM (authored by probinson).
Pass -fcrash-diagnostics-dir along to LLVM
Tue, Apr 6, 9:35 AM
probinson closed D99199: Make -fcrash-diagnostics-dir control the Windows (mini-)dump location.
Tue, Apr 6, 9:35 AM · Restricted Project, Restricted Project

Mon, Apr 5

probinson added a comment to D99199: Make -fcrash-diagnostics-dir control the Windows (mini-)dump location.

Ping.

Mon, Apr 5, 7:45 AM · Restricted Project, Restricted Project
probinson added inline comments to D99305: [docs] Document our norms around reverts.
Mon, Apr 5, 7:37 AM · Restricted Project

Thu, Apr 1

probinson added a reviewer for D99238: [DebugInfo] Enable the call site parameter feature by default: dblaikie.
Thu, Apr 1, 5:52 PM · Restricted Project, Restricted Project, debug-info
probinson added inline comments to D99048: [RFC][DebugInfo] Do not use the DBG_VALUE to calculate debug info of spill location.
Thu, Apr 1, 5:45 PM · Restricted Project
probinson added a comment to D99160: [X86][FastISEL] Support DW_TAG_call_site_parameter with FastISEL.

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 making sure debug info works well with that combination.

Thu, Apr 1, 5:44 PM · Restricted Project, Restricted Project, debug-info
probinson added a comment to D99692: [globalisel][unittests] Make it a compiler error to not call setUp() to avoid future rotten green tests.

I wasn't sure whether setUp() was supposed to be SetUp() or not. Good to have that confusion go away!

Thu, Apr 1, 6:36 AM · Restricted Project

Tue, Mar 30

probinson added inline comments to D99250: [DebugInfo] Fix the mismatching of C++ language tags and Dwarf versions..
Tue, Mar 30, 6:48 AM · debug-info, Restricted Project
probinson updated subscribers of D93203: [PS4] handle dllimport/export w.r.t vtables/rtti.

FTR, this looks okay from the PS4 owner's perspective, although IIRC @wristow did our original local patches.

Tue, Mar 30, 6:41 AM

Mon, Mar 29

probinson added a comment to D99199: Make -fcrash-diagnostics-dir control the Windows (mini-)dump location.

Addressed comments.

Mon, Mar 29, 8:28 AM · Restricted Project, Restricted Project
probinson updated the diff for D99199: Make -fcrash-diagnostics-dir control the Windows (mini-)dump location.

Remove ifdefs, tweak descriptions/comments.

Mon, Mar 29, 8:28 AM · Restricted Project, Restricted Project
probinson added inline comments to D99305: [docs] Document our norms around reverts.
Mon, Mar 29, 6:56 AM · Restricted Project

Fri, Mar 26

probinson added a comment to D99250: [DebugInfo] Fix the mismatching of C++ language tags and Dwarf versions..

Does anyone else have a DWARFv3 consumer they care about? (@aprantl and @probinson)

Does anyone have a DWARF consumer that changes significantly based on the language version (I guess lldb (@aprantl can you confirm?)? I doubt gdb does (yeah, it just treats all C++ versions the same)? how about Sony?)

I believe Sony is only using v4/v5 at this point. Regarding dialect controlling debugger behavior, I think it's unlikely, as distinguishing C++ dialects is new in v5, its adoption is pretty recent (at least for us), and we'd want behavior to be consistent across v4/v5 as much as possible. I'll verify that with our debugger guys.

Fri, Mar 26, 10:12 AM · debug-info, Restricted Project
probinson abandoned D98514: [RGT] Fix ASTMatchersTest so all assertions are executed.

Problem has gone away, test assertions are now executed.

Fri, Mar 26, 6:54 AM · Restricted Project

Thu, Mar 25

probinson added a comment to D99250: [DebugInfo] Fix the mismatching of C++ language tags and Dwarf versions..

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 in line no table.

Thu, Mar 25, 3:37 PM · debug-info, Restricted Project
probinson added a comment to D99250: [DebugInfo] Fix the mismatching of C++ language tags and Dwarf versions..

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.

Thu, Mar 25, 9:53 AM · debug-info, Restricted Project
probinson added a comment to D99250: [DebugInfo] Fix the mismatching of C++ language tags and Dwarf versions..

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?

Thu, Mar 25, 7:51 AM · debug-info, Restricted Project
probinson added a comment to D99305: [docs] Document our norms around reverts.

Please add words to the effect of: When re-landing a reverted patch, the commit message should be updated to indicate the problem that was addressed (bot failure, cite the PR, whatever).

Thu, Mar 25, 7:14 AM · Restricted Project

Wed, Mar 24

probinson accepted D99232: [Nomination] Adding new Google representative to security group.

Welcome to the ranks!

Wed, Mar 24, 11:48 AM · Restricted Project

Tue, Mar 23

probinson requested review of D99199: Make -fcrash-diagnostics-dir control the Windows (mini-)dump location.
Tue, Mar 23, 10:53 AM · Restricted Project, Restricted Project
probinson committed rGe150be612bf7: Document -fcrash-diagnostics-dir (authored by probinson).
Document -fcrash-diagnostics-dir
Tue, Mar 23, 10:47 AM
probinson closed D98873: Document -fcrash-diagnostics-dir.
Tue, Mar 23, 10:47 AM · Restricted Project

Fri, Mar 19

probinson committed rGfb4f6057a637: [RGT] Recode more unreachable assertions and tautologies (authored by probinson).
[RGT] Recode more unreachable assertions and tautologies
Fri, Mar 19, 9:19 AM
probinson added a reviewer for D98514: [RGT] Fix ASTMatchersTest so all assertions are executed: gribozavr.

+gribozavr who has been in this file most recently

Fri, Mar 19, 6:00 AM · Restricted Project

Thu, Mar 18

probinson requested review of D98873: Document -fcrash-diagnostics-dir.
Thu, Mar 18, 8:32 AM · Restricted Project

Wed, Mar 17

probinson committed rG05eeb6077a13: [RGT] RPCUtilsTest, replace un-executed EXPECT with unreachable (authored by probinson).
[RGT] RPCUtilsTest, replace un-executed EXPECT with unreachable
Wed, Mar 17, 7:38 AM
probinson closed D98518: [RGT] RPCUtilsTest, replace un-executed EXPECT with unreachable.
Wed, Mar 17, 7:38 AM · Restricted Project

Fri, Mar 12

probinson requested review of D98518: [RGT] RPCUtilsTest, replace un-executed EXPECT with unreachable.
Fri, Mar 12, 8:49 AM · Restricted Project
probinson updated the summary of D98514: [RGT] Fix ASTMatchersTest so all assertions are executed.
Fri, Mar 12, 8:21 AM · Restricted Project
probinson requested review of D98514: [RGT] Fix ASTMatchersTest so all assertions are executed.
Fri, Mar 12, 8:20 AM · Restricted Project

Mar 8 2021

probinson added a comment to D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete..

I started looking into some diffs of debug info in libc++ tests, but it's pretty hard to tell what's different - as far as I can see, there are just a bunch of __hash_value_types and __value_types.

Mar 8 2021, 6:46 AM · Restricted Project

Mar 2 2021

probinson added a comment to D96653: [FileCheck] Add neighboring annotations for -dump-input-filter=error.

As far as I know, it does. However, before CHECK-LABEL, a CHECK-NOT range is not the same as a CHECK range:

Mar 2 2021, 8:36 AM · Restricted Project

Mar 1 2021

probinson added a comment to D96653: [FileCheck] Add neighboring annotations for -dump-input-filter=error.

The strange part to me is that a preceding directive's search range ends at the end of the CHECK-LABEL match. It seems like it ought to end at the start of the CHECK-LABEL match.

Mar 1 2021, 9:33 AM · Restricted Project

Feb 26 2021

probinson requested review of D97566: [WIP][RGT] Rotten Green Test checking for LLVM and Clang unittests.
Feb 26 2021, 10:33 AM · Restricted Project, Restricted Project

Feb 25 2021

probinson added a comment to D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal.

GNU ld has a rule "start_/stop_ references from a live input section retain the associated C identifier name sections", which LLD may change in the future (D96914).

The phrasing "may change" implies LLD could eliminate the rule; in fact D96914 will only add a way to opt-out of the rule. You can't eliminate the rule entirely without breaking a lot of useful cases.

Reworded this sentence. D96914 does add a way to opt-out of the rule. In the future (when toolchains are more mature) we may try. My internal tests and Linux kernel tests are try to state that we may not have many dependent cases.

Feb 25 2021, 10:08 AM · Restricted Project
probinson added a comment to D97446: Change some addUsedGlobal to addUsedOrCompilerUsedGlobal.

GNU ld has a rule "start_/stop_ references from a live input section retain the associated C identifier name sections",

which LLD may change in the future (D96914).

Feb 25 2021, 8:22 AM · Restricted Project

Feb 24 2021

probinson added a comment to D96838: Add GNU attribute 'retain'.

Aha; attribute used *by itself* is not sufficient to preserve sections in the output. But the __start_/__stop_ symbols implicitly create a reference to each of the named sections, and that implicit reference can preserve them in the output (assuming gc roots etc).
So, the idea is that attribute retain can be used *instead* of the __start_/__stop_ symbols, to preserve sections in the output (with the advantage that it will work even for sections that do not have a C-identifier name).

Feb 24 2021, 8:46 AM · Restricted Project

Feb 23 2021

probinson added a comment to D96838: Add GNU attribute 'retain'.

For ELF, used does not retain the entity, regardless of this patch.

Feb 23 2021, 12:58 PM · Restricted Project
probinson added inline comments to D96838: Add GNU attribute 'retain'.
Feb 23 2021, 10:41 AM · Restricted Project
probinson accepted D97242: Add more historic DWARF vendor extensions.

A few typos. Nice to have these!

Feb 23 2021, 8:28 AM · Restricted Project, debug-info

Feb 12 2021

probinson committed rG98754e290908: [RGT][GlobalIsel] Add missing setUp() calls to legalizer unittests (authored by probinson).
[RGT][GlobalIsel] Add missing setUp() calls to legalizer unittests
Feb 12 2021, 10:47 AM
probinson closed D95257: [RGT][GlobalISel] Add missing setUp() calls to legalizer unittests.
Feb 12 2021, 10:47 AM · Restricted Project

Feb 10 2021

probinson added a comment to D96354: Avoid conflicts between debug-info and pseudo-probe profiling.

the driver had a redundant pass-through of the option

Feb 10 2021, 7:54 AM · Restricted Project
probinson committed rG5ea2d4fa4811: Avoid conflicts between debug-info and pseudo-probe profiling (authored by probinson).
Avoid conflicts between debug-info and pseudo-probe profiling
Feb 10 2021, 7:35 AM
probinson closed D96354: Avoid conflicts between debug-info and pseudo-probe profiling.
Feb 10 2021, 7:35 AM · Restricted Project

Feb 9 2021

probinson requested review of D96354: Avoid conflicts between debug-info and pseudo-probe profiling.
Feb 9 2021, 9:50 AM · Restricted Project
probinson added a comment to D96274: [clang][cli] Generate and round-trip Diagnostic options.

I don't claim to be very familiar with this area but "round-trip" and "test" would make me think those bits should be in a lit test or unittest. As it is, it's not obvious what is functional and what is testing here.
Possibly I am misunderstanding something fundamental about how these options work?

Feb 9 2021, 6:29 AM · Restricted Project

Feb 8 2021

probinson added a comment to D96144: [ELF] Add --dwarf32-before-dwarf64 to sort DWARF32 input sections before DWARF64.

I've re-read a fair amount of the previous llvm-dev thread.
One goal of doing a patch like this was to collect some performance data; do we have that? In particular, performance data comparing link times with the option default-on versus default-off, so we can determine whether the difference is small enough that we should just do this processing unconditionally.

Feb 8 2021, 7:52 AM · debug-info, Restricted Project, lld

Feb 5 2021

probinson added a reviewer for D95257: [RGT][GlobalISel] Add missing setUp() calls to legalizer unittests: qcolombet.
Feb 5 2021, 9:15 AM · Restricted Project
probinson updated the diff for D95257: [RGT][GlobalISel] Add missing setUp() calls to legalizer unittests.

Modified tests as recommended by Quentin on llvm-dev; I hope I got it right. Wasn't clear if the intent was to modify *all* tests in the file the same way (adding B.setInstr(foo)).

Feb 5 2021, 9:14 AM · Restricted Project
probinson committed rGa0749f9bcc7c: [RGT][ProfileData] Correct a test assertion (authored by probinson).
[RGT][ProfileData] Correct a test assertion
Feb 5 2021, 6:55 AM
probinson closed D95258: [RGT][ProfileData] Correct a test assertion.
Feb 5 2021, 6:55 AM · Restricted Project

Feb 4 2021

probinson committed rG144ca1e5bc1c: [PS4] Allow triple to reflect the new company name. (authored by probinson).
[PS4] Allow triple to reflect the new company name.
Feb 4 2021, 9:43 AM
probinson accepted D96014: Add 'LLVM_DEFAULT_TARGET_TRIPLE' to the documented list of CMake variables.

Seems reasonable, provided you correct the language to be about building a cross-compiler rather than actually cross-compiling.

Feb 4 2021, 8:30 AM · Restricted Project
probinson added a comment to D95258: [RGT][ProfileData] Correct a test assertion.

Ping

Feb 4 2021, 8:05 AM · Restricted Project

Jan 29 2021

probinson added a comment to D95257: [RGT][GlobalISel] Add missing setUp() calls to legalizer unittests.

I asked about this on llvm-dev but it seemed worth reporting here as well.
Some of the differences between the expected output (per the FileCheck directives) and the actual output appear to be little more than reordering of the instructions. However, oddly, in some cases the use of a value might precede its definition. For example, in NarrowSEXTINREG, the test is looking for this:

CHECK: [[T0:%[0-9]+]]:_(s16) = G_TRUNC
CHECK: [[T1:%[0-9]+]]:_(s10) = G_TRUNC [[T0]]:_s(16)
CHECK: [[T2:%[0-9]+]]:_(s10) = G_SEXT_INREG [[T1]]:_, 8
CHECK: [[T3:%[0-9]+]]:_(s16) = G_SEXT [[T2]]:_(s10)

But, what I see is this:

%5:_(s16) = G_SEXT %7:_(s10)
%4:_(s16) = G_TRUNC %0:_(s64)
%7:_(s10) = G_SEXT_INREG %6:_, 8
%6:_(s10) = G_TRUNC %4:_(s16)

So the SEXT uses the result of SEXT_INREG, which uses the result of the second TRUNC; these are all forward references. That doesn't seem correct; uses before defs? If there's a problem with instruction ordering, fixing that seems like its own issue, and would probably make many of the failures I'm seeing just go away. (Global ISel is very much not my area, so I'd appreciate it if someone else could figure out the ordering question.)

Jan 29 2021, 9:57 AM · Restricted Project

Jan 25 2021

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.

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

Jan 22 2021

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
Jan 22 2021, 3:08 PM
probinson closed D95259: [RGT][TextAPI] Remove a zero-trip loop and the assertions within it.
Jan 22 2021, 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
Jan 22 2021, 3:05 PM
probinson closed D95256: [RGT] Don't use EXPECT* macros in a test subprcess that exits by signalling..
Jan 22 2021, 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
Jan 22 2021, 3:04 PM
probinson closed D95255: [RGT][ADT] Remove test assertion that will not be executed..
Jan 22 2021, 3:04 PM · Restricted Project
probinson added inline comments to D95257: [RGT][GlobalISel] Add missing setUp() calls to legalizer unittests.
Jan 22 2021, 1:14 PM · Restricted Project
probinson requested review of D95259: [RGT][TextAPI] Remove a zero-trip loop and the assertions within it.
Jan 22 2021, 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.

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

Jan 21 2021

probinson added inline comments to D95158: Use DataExtractor to decode SLEB128 in android_relas..
Jan 21 2021, 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.

Jan 21 2021, 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.

Jan 21 2021, 6:25 AM · Restricted Project

Jan 19 2021

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

Catching up on this, sorry...

Jan 19 2021, 8:54 AM · Restricted Project

Jan 11 2021

probinson committed rG1f9c29228cec: [FastISel] NFC: Clean up unnecessary bookkeeping (authored by probinson).
[FastISel] NFC: Clean up unnecessary bookkeeping
Jan 11 2021, 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
Jan 11 2021, 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
Jan 11 2021, 8:33 AM
probinson closed D91734: [FastISel] Flush local value map on every instruction.
Jan 11 2021, 8:33 AM · Restricted Project, Restricted Project, debug-info
probinson committed rGe5eb5c8a7f30: NFC: Use -LABEL more (authored by probinson).
NFC: Use -LABEL more
Jan 11 2021, 8:24 AM

Jan 6 2021

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.

Jan 6 2021, 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)

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

Jan 5 2021

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.

Jan 5 2021, 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).

Jan 5 2021, 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