Page MenuHomePhabricator

dsanders (Daniel Sanders)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 19 2013, 3:30 PM (346 w, 1 d)

Recent Activity

Today

dsanders committed rG1adeeabb79af: Add MIR-level debugify with only locations support for now (authored by dsanders).
Add MIR-level debugify with only locations support for now
Tue, Apr 7, 4:55 PM
dsanders closed D77446: Add MIR-level debugify with only locations support for now.
Tue, Apr 7, 4:55 PM · Restricted Project
dsanders added a comment to D77446: Add MIR-level debugify with only locations support for now.
In D77446#1967893, @vsk wrote:

Thanks, lgtm!

Later on, I think we'll want to insert DBG_VALUE insts as well, but this looks very useful as-is.

Tue, Apr 7, 4:54 PM · Restricted Project
dsanders updated the diff for D77446: Add MIR-level debugify with only locations support for now.

Add test case

Tue, Apr 7, 1:03 PM · Restricted Project

Yesterday

dsanders updated the diff for D77576: [globalisel] Add lost debug locations verifier.

C++ mode comment
Use ore::NV
Fix build errors in legalizer unit test

Mon, Apr 6, 7:06 PM · Restricted Project
dsanders updated the summary of D77576: [globalisel] Add lost debug locations verifier.
Mon, Apr 6, 6:01 PM · Restricted Project
dsanders added inline comments to D77576: [globalisel] Add lost debug locations verifier.
Mon, Apr 6, 6:01 PM · Restricted Project
dsanders updated the diff for D77446: Add MIR-level debugify with only locations support for now.

Add doxygen comment
Remove MF.dump()

Mon, Apr 6, 4:55 PM · Restricted Project
dsanders committed rGf27cea721e58: Add way to omit debug-location from MIR output (authored by dsanders).
Add way to omit debug-location from MIR output
Mon, Apr 6, 4:23 PM
dsanders closed D77575: [debugify] Add way to omit debug-location from MIR output.
Mon, Apr 6, 4:23 PM · Restricted Project
dsanders added inline comments to D77446: Add MIR-level debugify with only locations support for now.
Mon, Apr 6, 4:22 PM · Restricted Project
dsanders committed rG35b7b0851b19: Allow MachineFunction to obtain non-const Function (to enable MIR-level… (authored by dsanders).
Allow MachineFunction to obtain non-const Function (to enable MIR-level…
Mon, Apr 6, 3:50 PM
dsanders closed D77439: Allow MachineFunction to obtain non-const Function (to enable MIR-level debugify).
Mon, Apr 6, 3:49 PM · Restricted Project
dsanders committed rG15f7bc785725: Add option to limit Debugify to locations (omitting variables) (authored by dsanders).
Add option to limit Debugify to locations (omitting variables)
Mon, Apr 6, 3:17 PM
dsanders closed D77438: Add option to limit Debugify to locations (omitting variables).
Mon, Apr 6, 3:17 PM · Restricted Project
dsanders added a comment to D77575: [debugify] Add way to omit debug-location from MIR output.

In lieu of a proper pass that strips debug info

What is it that you need? Something like llvm::stripDebugInfo() (from DebugInfo.cpp) but for MIR? That should be very easy to adapt.

Mon, Apr 6, 2:43 PM · Restricted Project
dsanders added a comment to D77575: [debugify] Add way to omit debug-location from MIR output.
In D77575#1964990, @vsk wrote:

Why not generalize this to something like -mir-print-debug-info, which, when set to false, makes the MIR printer stop printing DBG_ meta instructions as well? Then you might not need D77438.

Mon, Apr 6, 1:37 PM · Restricted Project
dsanders added a comment to D77575: [debugify] Add way to omit debug-location from MIR output.

I haven't added a test case for this as D77576 covers it

Mon, Apr 6, 12:30 PM · Restricted Project
dsanders updated the diff for D77438: Add option to limit Debugify to locations (omitting variables).

+ test case

Mon, Apr 6, 12:30 PM · Restricted Project
dsanders added inline comments to D77576: [globalisel] Add lost debug locations verifier.
Mon, Apr 6, 11:28 AM · Restricted Project
dsanders added reviewers for D77576: [globalisel] Add lost debug locations verifier: aemerson, paquette, arsenm.
Mon, Apr 6, 11:25 AM · Restricted Project
dsanders created D77576: [globalisel] Add lost debug locations verifier.
Mon, Apr 6, 11:25 AM · Restricted Project
dsanders added a child revision for D77446: Add MIR-level debugify with only locations support for now: D77576: [globalisel] Add lost debug locations verifier.
Mon, Apr 6, 11:25 AM · Restricted Project
dsanders added a child revision for D77575: [debugify] Add way to omit debug-location from MIR output: D77576: [globalisel] Add lost debug locations verifier.
Mon, Apr 6, 11:25 AM · Restricted Project
dsanders created D77575: [debugify] Add way to omit debug-location from MIR output.
Mon, Apr 6, 11:25 AM · Restricted Project

Fri, Apr 3

dsanders added a reviewer for D77438: Add option to limit Debugify to locations (omitting variables): vsk.
Fri, Apr 3, 5:53 PM · Restricted Project
dsanders added a reviewer for D77439: Allow MachineFunction to obtain non-const Function (to enable MIR-level debugify): vsk.
Fri, Apr 3, 5:53 PM · Restricted Project
dsanders created D77446: Add MIR-level debugify with only locations support for now.
Fri, Apr 3, 5:53 PM · Restricted Project
dsanders added a child revision for D77438: Add option to limit Debugify to locations (omitting variables): D77446: Add MIR-level debugify with only locations support for now.
Fri, Apr 3, 5:53 PM · Restricted Project
dsanders added a child revision for D77439: Allow MachineFunction to obtain non-const Function (to enable MIR-level debugify): D77446: Add MIR-level debugify with only locations support for now.
Fri, Apr 3, 5:53 PM · Restricted Project
dsanders created D77439: Allow MachineFunction to obtain non-const Function (to enable MIR-level debugify).
Fri, Apr 3, 4:49 PM · Restricted Project
dsanders created D77438: Add option to limit Debugify to locations (omitting variables).
Fri, Apr 3, 4:49 PM · Restricted Project

Wed, Apr 1

dsanders committed rGe65e677ee4ee: [globalisel][legalizer] Fix DebugLoc bugs caught by a prototype lost-location… (authored by dsanders).
[globalisel][legalizer] Fix DebugLoc bugs caught by a prototype lost-location…
Wed, Apr 1, 12:57 PM

Tue, Mar 31

dsanders added inline comments to D76640: [GlobalISel] Combine (x op 0) -> x for operations with a right identity of 0.
Tue, Mar 31, 10:33 AM · Restricted Project
dsanders accepted D77103: Add a new -fglobal-isel option and make -fexperimental-isel an alias for it..

LGTM

Tue, Mar 31, 9:57 AM · Restricted Project, Restricted Project

Mon, Mar 23

dsanders accepted D73200: GlobalISel: Add computeKnownBitsForTargetInstr.

LGTM after syncing up with @foad's change to use countLeadingOnes() in DAGISel

Mon, Mar 23, 9:48 AM · Restricted Project
dsanders accepted D76579: GlobalISel: Prepare to allow other target unit tests.

LGTM aside from that orphaned comment

Mon, Mar 23, 9:48 AM

Fri, Mar 20

dsanders accepted D76494: [GlobalISel] support widen unmerge if WideTy > SrcTy.

LGTM. I expect that it's just that we didn't encounter it or think it could occur

Fri, Mar 20, 11:55 AM · Restricted Project

Thu, Mar 19

dsanders added inline comments to D69808: [RISCV GlobalISel] Add lowerReturn for calling conv..
Thu, Mar 19, 10:54 AM · Restricted Project

Wed, Mar 18

dsanders accepted D76339: [GlobalISel] Port some basic undef combines from DAGCombiner.cpp.

I was going to ask you to use the custom predicate feature but it doesn't appear to be there. I could have sworn I'd committed that before I had to temporarily prioritize other work. It doesn't seem to have been committed and reverted without me noticing either. Oh well, LGTM with the all_combine change and I'll migrate it to custom predicates once I've actually landed that.

Wed, Mar 18, 10:52 AM · Restricted Project

Mon, Mar 16

dsanders added inline comments to D75994: [GlobalISel] add additional lowering support for G_INSERT.
Mon, Mar 16, 10:20 AM · Restricted Project
dsanders accepted D74988: Propagate MIFlags in table gen.

LGTM. Sorry for the wait, it shouldn't have taken me this long really as I'd already reviewed the same patch downstream.

Mon, Mar 16, 9:48 AM · Restricted Project

Feb 21 2020

dsanders added a comment to D74943: [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time.

I've spoken to Quentin offline and it seems we're not in as much disagreement as it appears. The main issues are:

  1. Non-SSA passes aren't included in the plan (I'm focused on IRTranslator to InstructionSelector which are all SSA)
  2. Detecting when the contract is violated
  3. Not having to write maintenance code

I'll give some more thought to those before I (eventually) make the proposal. I think I have 3. covered, 2. is broadly ok but certainly has room for improvement. For 1. I suggested an off-the-cuff solution that abstracted the cache key to allow implementations to compute a value number by some means instead of directly using the vreg number but it needs some proper thought.

Feb 21 2020, 2:57 PM · Restricted Project
dsanders added a comment to D74943: [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time.

That's true (although you'd need a third vreg with no defs/uses to do it) but as you say there's no reason to do so.

That's my point. Even if there is no reason to do so, it is possible.
What I am saying is a mechanism that solely relies on registers not changing value is not bug free.

Feb 21 2020, 1:06 PM · Restricted Project
dsanders added a comment to D74943: [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time.

My long term plan for the intra/inter-pass cache is based about caching based on the register too.

That seems fragile to do that generally. I agree that is less likely that a register changes, but it is not programmatically impossible. E.g., we could imagine that register numbers get recycle and then we would hit a cache entry that was not intended for a register.

Bottom line, whatever caching we do, it will have to be bound to some kind of observer in my opinion.

We don't recycle vreg numbers today and I'm not aware of any plans to do so. There's also no API to delete a vreg. It's technically possible to change the type but that's potentially dangerous.

At the moment, everyone calls createVirtualRegister() when they need one that didn't exist before and the pre-existing defs at worst become unused by some/all of their uses. A vreg def is never replaced by the def of an instruction that isn't computing (or finishing computing) the same value.

You could swap two unrelated registers with the asme type by uisng replaceRegWith, though I don't know why you would ever do that

Feb 21 2020, 11:58 AM · Restricted Project
dsanders added a comment to D74943: [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time.

My long term plan for the intra/inter-pass cache is based about caching based on the register too.

That seems fragile to do that generally. I agree that is less likely that a register changes, but it is not programmatically impossible. E.g., we could imagine that register numbers get recycle and then we would hit a cache entry that was not intended for a register.

Bottom line, whatever caching we do, it will have to be bound to some kind of observer in my opinion.

Feb 21 2020, 11:48 AM · Restricted Project
dsanders accepted D74943: [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time.

LGTM

Feb 21 2020, 11:27 AM · Restricted Project

Feb 19 2020

dsanders accepted D74803: TableGen: Fix logic for default operands.

I haven't dug very deep into this but it makes sense to me to be reading the InstInfo and as Simon notes, they should be identical for the cases that already work. On that basis LGTM

Feb 19 2020, 12:14 PM · Restricted Project
dsanders updated the diff for D74796: [tablegen] Add !iscomplete(Value).

Additional comments

Feb 19 2020, 11:18 AM · Restricted Project
dsanders added inline comments to D74796: [tablegen] Add !iscomplete(Value).
Feb 19 2020, 11:18 AM · Restricted Project

Feb 18 2020

dsanders retitled D74796: [tablegen] Add !iscomplete(Value) from Add !iscomplete(Value) to [tablegen] Add !iscomplete(Value).
Feb 18 2020, 2:53 PM · Restricted Project
dsanders created D74796: [tablegen] Add !iscomplete(Value).
Feb 18 2020, 2:37 PM · Restricted Project
dsanders committed rG2c8ee5329bb1: Fix assertion on `!eq(?, 0)` (authored by dsanders).
Fix assertion on `!eq(?, 0)`
Feb 18 2020, 2:08 PM

Feb 17 2020

dsanders accepted D74715: GlobalISel: Allow running localizer earlier.

LGTM

Feb 17 2020, 11:07 AM · Restricted Project

Feb 3 2020

dsanders accepted D73875: [DAG] OptLevelChanger - fix uninitialized variable analyzer warning (PR44471).

LGTM

Feb 3 2020, 11:57 AM · Restricted Project

Jan 29 2020

dsanders added inline comments to D73200: GlobalISel: Add computeKnownBitsForTargetInstr.
Jan 29 2020, 10:53 AM · Restricted Project

Jan 24 2020

dsanders accepted D73159: ARM64: Debug info for structure argument missing DW_AT_location.

LGTM

Jan 24 2020, 10:43 AM · Restricted Project, debug-info

Jan 23 2020

dsanders added inline comments to D73159: ARM64: Debug info for structure argument missing DW_AT_location.
Jan 23 2020, 3:21 PM · Restricted Project, debug-info

Jan 22 2020

dsanders added inline comments to D73159: ARM64: Debug info for structure argument missing DW_AT_location.
Jan 22 2020, 12:42 PM · Restricted Project, debug-info

Jan 14 2020

dsanders accepted D72726: TableGen/GlobalISel: Don't take reference to temporary values.

LGTM

Jan 14 2020, 1:38 PM · Restricted Project
dsanders accepted D72725: TableGen/GlobalISel: Don't reconstruct CodeGenRegBank.

LGTM

Jan 14 2020, 1:38 PM · Restricted Project

Jan 13 2020

dsanders added a comment to D72574: [PowerPC][Future] Add pld and pstd to future CPU.

I'm not convinced you really need the FixedLenDecoderEmitter.cpp changes. PPCInstrFormats.td has bits<64> Inst in the I2 class, but PPCDisassembler.cpp has InsnType as being uint32_t . InsnType needs to be able to hold your biggest instruction for the generated code to work correctly. I believe that if you changed:

uint32_t Inst = ...;

in PPCDisassembler::getInstruction(), to:

uint64_t Inst = ...;

then you will no longer need to change FixedLenDecoderEmitter.cpp as you'll be able to use shifts of up to 63.

Jan 13 2020, 4:23 PM · Restricted Project, Restricted Project
dsanders updated the diff for D72636: Cancelling out G_MERGE_VALUES/G_UNMERGE pairs sometimes needs a copy.

Fix the remaining small issues

Jan 13 2020, 3:35 PM · Restricted Project
dsanders updated the diff for D72636: Cancelling out G_MERGE_VALUES/G_UNMERGE pairs sometimes needs a copy.

+Testcase

Jan 13 2020, 3:26 PM · Restricted Project
dsanders committed rGa0f4600f4f0e: Rework be15dfa88fb1 such that it works with GlobalISel which doesn't use EVT (authored by dsanders).
Rework be15dfa88fb1 such that it works with GlobalISel which doesn't use EVT
Jan 13 2020, 12:28 PM
dsanders closed D72309: Rework be15dfa88fb1 such that it works with GlobalISel which doesn't use EVT.
Jan 13 2020, 12:28 PM · Restricted Project
dsanders created D72636: Cancelling out G_MERGE_VALUES/G_UNMERGE pairs sometimes needs a copy.
Jan 13 2020, 11:59 AM · Restricted Project

Jan 10 2020

dsanders updated the diff for D72309: Rework be15dfa88fb1 such that it works with GlobalISel which doesn't use EVT.

isInteger -> isIntegerLike and comment on pointers needing to use isIntegerLike

Jan 10 2020, 11:37 AM · Restricted Project
dsanders added a comment to D72228: [MIPS][ELF] Use PC-relative relocations in .eh_frame when possible.

I see you've added me as a reviewer but I haven't worked on MIPS for a few years so I'm not really familiar with the current state of their linkers and binaries. AFAIK @atanasyan is still actively working on MIPS so hopefully he can take a look or bring in a colleague who can.
FWIW, this sounds like a good change and the code looks good to me but I think you should get another LGTM from someone more up to date on MIPS.

Jan 10 2020, 10:50 AM · Restricted Project
dsanders committed rGa5230ac10b0d: Update the attribution policy to use the 'Author' property of a git commit (authored by dsanders).
Update the attribution policy to use the 'Author' property of a git commit
Jan 10 2020, 10:32 AM
dsanders closed D72468: Update the attribution policy to use the 'Author' property of a git commit.
Jan 10 2020, 10:32 AM · Restricted Project
dsanders added a comment to D72468: Update the attribution policy to use the 'Author' property of a git commit.

Thanks everyone. Landed in a5230ac10b0

Jan 10 2020, 10:31 AM · Restricted Project
dsanders updated subscribers of D69836: [MIR] Target specific MIR formating and parsing.
Jan 10 2020, 10:31 AM · Restricted Project

Jan 9 2020

dsanders updated the diff for D72468: Update the attribution policy to use the 'Author' property of a git commit.

Add a bit about asking someone to commit for you so that we can document
providing the information needed for the Author property

Jan 9 2020, 1:32 PM · Restricted Project
dsanders added inline comments to D72468: Update the attribution policy to use the 'Author' property of a git commit.
Jan 9 2020, 1:32 PM · Restricted Project
dsanders added inline comments to D72468: Update the attribution policy to use the 'Author' property of a git commit.
Jan 9 2020, 1:13 PM · Restricted Project
dsanders updated the diff for D72468: Update the attribution policy to use the 'Author' property of a git commit.

Small changes for review comments

Jan 9 2020, 1:13 PM · Restricted Project
dsanders accepted D72464: GlobalISel: Don't assert on MoreElements creating vectors.

LGTM

Jan 9 2020, 10:52 AM · Restricted Project
dsanders created D72468: Update the attribution policy to use the 'Author' property of a git commit.
Jan 9 2020, 10:42 AM · Restricted Project

Jan 8 2020

dsanders committed rGde3d0ee023cb: Revert "Revert "[MIR] Target specific MIR formating and parsing"" (authored by dsanders).
Revert "Revert "[MIR] Target specific MIR formating and parsing""
Jan 8 2020, 8:06 PM
dsanders added a reverting change for rG71d64f72f934: Revert "[MIR] Target specific MIR formating and parsing": rGde3d0ee023cb: Revert "Revert "[MIR] Target specific MIR formating and parsing"".
Jan 8 2020, 8:06 PM
dsanders added a comment to D69836: [MIR] Target specific MIR formating and parsing.

Temporarily reverted in 71d64f72f934631aa2f for now.

Jan 8 2020, 8:05 PM · Restricted Project
dsanders committed rG3ef05d85be8c: [MIR] Target specific MIR formating and parsing (authored by pguo).
[MIR] Target specific MIR formating and parsing
Jan 8 2020, 6:52 PM
dsanders committed rG5ab6fa7b7011: Revert "[MIR] Target specific MIR formating and parsing" (authored by dsanders).
Revert "[MIR] Target specific MIR formating and parsing"
Jan 8 2020, 6:52 PM
dsanders added a reverting change for rGbe841f89d001: [MIR] Target specific MIR formating and parsing: rG5ab6fa7b7011: Revert "[MIR] Target specific MIR formating and parsing".
Jan 8 2020, 6:52 PM
dsanders committed rGbe841f89d001: [MIR] Target specific MIR formating and parsing (authored by pguo).
[MIR] Target specific MIR formating and parsing
Jan 8 2020, 6:43 PM
dsanders closed D69836: [MIR] Target specific MIR formating and parsing.
Jan 8 2020, 6:43 PM · Restricted Project

Jan 7 2020

dsanders committed rG13922f3e9d0c: Fix warnings as errors that occur on sanitizer-x86_64-linux (authored by dsanders).
Fix warnings as errors that occur on sanitizer-x86_64-linux
Jan 7 2020, 4:03 PM
dsanders committed rG39c05703a6ca: [gicombiner] Correct 64f1bb5cd2c to account for MSVC's %p format (authored by dsanders).
[gicombiner] Correct 64f1bb5cd2c to account for MSVC's %p format
Jan 7 2020, 12:59 PM
dsanders added a comment to D69152: [gicombiner] Add GIMatchTree and use it for the code generation.

This broke tests on Windows: http://45.33.8.238/win/5239/step_11.txt

Probably the "%p doesn't include 0x on Windows" thing again.

Jan 7 2020, 11:41 AM · Restricted Project
dsanders committed rG1d94fb211187: [gicombiner] Add GIMatchTree and use it for the code generation (authored by dsanders).
[gicombiner] Add GIMatchTree and use it for the code generation
Jan 7 2020, 11:13 AM
dsanders closed D69152: [gicombiner] Add GIMatchTree and use it for the code generation.
Jan 7 2020, 11:13 AM · Restricted Project
dsanders added a comment to D72309: Rework be15dfa88fb1 such that it works with GlobalISel which doesn't use EVT.

Could we invert the boolean flag to be isFloat? I fear that calling it isInteger, will lead the the same problems that I tried to fix in that commit (calling .isInteger() returns false for pointers).

Jan 7 2020, 10:14 AM · Restricted Project

Jan 6 2020

dsanders updated the diff for D72309: Rework be15dfa88fb1 such that it works with GlobalISel which doesn't use EVT.

Corrected patch

Jan 6 2020, 6:42 PM · Restricted Project
dsanders created D72309: Rework be15dfa88fb1 such that it works with GlobalISel which doesn't use EVT.
Jan 6 2020, 6:24 PM · Restricted Project
dsanders accepted D72273: Make check-llvm run 50% faster on macOS, 18% faster on Windows..

LGTM. Thanks!

Jan 6 2020, 9:56 AM · Restricted Project

Jan 3 2020

dsanders reopened D69152: [gicombiner] Add GIMatchTree and use it for the code generation.
Jan 3 2020, 7:26 PM · Restricted Project
dsanders committed rG5d304d68dd5f: Revert "[gicombiner] Add GIMatchTree and use it for the code generation" (authored by dsanders).
Revert "[gicombiner] Add GIMatchTree and use it for the code generation"
Jan 3 2020, 6:22 PM
dsanders added a reverting change for rG64f1bb5cd2c6: [gicombiner] Add GIMatchTree and use it for the code generation: rG5d304d68dd5f: Revert "[gicombiner] Add GIMatchTree and use it for the code generation".
Jan 3 2020, 6:21 PM
dsanders added a reverting change for rG77d4b5f5feff: [gicombiner] Correct 64f1bb5cd2c to account for MSVC's %p format: rG5d304d68dd5f: Revert "[gicombiner] Add GIMatchTree and use it for the code generation".
Jan 3 2020, 6:21 PM