MatzeB (Matthias Braun)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 13 2013, 3:10 PM (275 w, 1 d)

Recent Activity

Fri, Nov 9

MatzeB added a comment to D52010: RegAllocFast: Rewrite and improve.

Some notes on what is left here:

  • Change allocation direction to go from end to begin of a basic block.
  • Change debug value handling: The tricky thing with the changed direction is that we dealing with dangling DBG_VALUE instructions (they have a vreg use after the last non-debug use of a vreg) gets trickier. They are rare but occur in several unittests so we now track them separately in the code and perform a forward walk each time after the vreg is allocation by the real last use to see whether the register reached the dangling DBG_VALUE.
  • This also simplifies DBG_VALUE handling, to always referenvce stack slots in case of spills as we now place spills immediately adjacent to the definition of a value/register.
  • Generally rewrote allocateInstruction to be hopefolly somewhat simpler
  • Added some heuristic for tricky inline assembly early clobber situations: If we detect a tricky situation now, we will take all def and early-clobber like operands and sort them by the size of the register class so that we can assign tricky cases more consistently and are more likely to be successfull (this was necessary to fix some unittests that were lucky with the old allocation algorithm but unlucky with the new one. With the new heuristic we should be less dependent on luck).
Fri, Nov 9, 4:47 PM
MatzeB updated the diff for D52010: RegAllocFast: Rewrite and improve.
  • Simpler patch now that more things have been split up into separate review and NFC patches.
Fri, Nov 9, 4:42 PM
MatzeB added a comment to D52010: RegAllocFast: Rewrite and improve.
  • I split up two more NFC commits: rL346575 and rL346576.
  • I split several step by step patches to ease review (note that I did not bother to fix hundreds of testcases for the intermediate steps, so I will not commit them independently): D54364, D54365, D54366, D54367, D54368
Fri, Nov 9, 4:41 PM
MatzeB added a comment to D54368: RegAllocFast: Record internal state based on register units.

This is part of my rewrite regallocfast series. See also D52010

Fri, Nov 9, 4:38 PM
MatzeB added reviewers for D54368: RegAllocFast: Record internal state based on register units: kparzysz, myatsina, javed.absar, arsenm.
Fri, Nov 9, 4:38 PM
MatzeB created D54368: RegAllocFast: Record internal state based on register units.
Fri, Nov 9, 4:37 PM
MatzeB created D54367: RegAllocFast: Improve hinting heuristic.
Fri, Nov 9, 4:36 PM
MatzeB added a comment to D54366: RegAllocFast: Add heuristic to detect values not live-out of a block.

This is part of my rewrite regallocfast series. See also D52010

Fri, Nov 9, 4:33 PM
MatzeB created D54366: RegAllocFast: Add heuristic to detect values not live-out of a block.
Fri, Nov 9, 4:33 PM
MatzeB added a comment to D54365: RegAllocFast: Remove early selection loop, the spill calculation will report cost 0 anyway for free regs.

This is part of my regallocfast rewrite series, see also D52010

Fri, Nov 9, 4:31 PM
MatzeB created D54365: RegAllocFast: Remove early selection loop, the spill calculation will report cost 0 anyway for free regs.
Fri, Nov 9, 4:31 PM
MatzeB created D54364: RegAllocFast: Do not allocate registers for undef uses.
Fri, Nov 9, 4:28 PM
MatzeB added reviewers for D54137: AArch64: Fix invalid CCMP emission: t.p.northover, efriedma, greened, evandro, gberry.

Maybe you can give it at least some cursory review?

Fri, Nov 9, 3:24 PM

Thu, Nov 8

MatzeB accepted D54203: Use template instead of void* -based type erasure in MachineRegistry.

Take my "meh" as LGTM if nobody objects in the next couple days.

Thu, Nov 8, 11:30 AM
MatzeB added a comment to D54203: Use template instead of void* -based type erasure in MachineRegistry.

The whole registry system isn't used much. The whole machine seems a little overengineered in retrospect (there only ever is a single listener for example). But making it typesafe seems sensible.

Thu, Nov 8, 11:30 AM

Wed, Nov 7

MatzeB updated the diff for D54137: AArch64: Fix invalid CCMP emission.

And fix one more variable name typo (sorry for all the updates).

Wed, Nov 7, 4:42 PM
MatzeB updated the diff for D54137: AArch64: Fix invalid CCMP emission.

And fix one more variable name typo (sorry for all the updates).

Wed, Nov 7, 4:42 PM
MatzeB updated the diff for D54137: AArch64: Fix invalid CCMP emission.

Tweak some more comments.

Wed, Nov 7, 4:39 PM
MatzeB updated the diff for D54137: AArch64: Fix invalid CCMP emission.

fix typos

Wed, Nov 7, 4:30 PM
MatzeB updated the diff for D54137: AArch64: Fix invalid CCMP emission.
  • Added two more tests
  • Revamped explanation of the transformation. It's better but I fear still very hard to crasp. Oh well it's as good as I can make it right now...
Wed, Nov 7, 4:29 PM
MatzeB added a comment to D54137: AArch64: Fix invalid CCMP emission.

@mstorsjo could you check if this fixes your original problem?

Wed, Nov 7, 4:10 PM
MatzeB updated the diff for D54137: AArch64: Fix invalid CCMP emission.

After pondering about this again, this should be the correct fix.

Wed, Nov 7, 4:10 PM

Tue, Nov 6

MatzeB added a comment to D52010: RegAllocFast: Rewrite and improve.

I extracted a couple of obvious cleanups to reduce the code churn here. They landed in rL346287, rL346288, rL346289, rL346296, rL346297, rL346298
(will update and split the diff here soon)

Tue, Nov 6, 11:01 PM
MatzeB added a comment to D52010: RegAllocFast: Rewrite and improve.

@abeserminji: Aleksandar, could you take a look at the RegAllocFast::reloadAtBegin? It looks like a necessity of using the rafast-ignore-missing-defs option is caused by the rL336328 commit.

Tue, Nov 6, 10:14 PM
MatzeB added reviewers for D52010: RegAllocFast: Rewrite and improve: atanasyan, smaksimovic, petarj.

@mips target owners: Please take a look at the unittests where I had to use -rafast-ignore-missing-defs to workaround invalid machine code. Those tests use a virtual register before it is defined (the old regalloc fast used to just ignore this in some cases).

Tue, Nov 6, 3:10 PM
MatzeB updated the diff for D52010: RegAllocFast: Rewrite and improve.
  • Adapted all the unit tests
  • Slight tuning of register hinting code
  • Improved allocation heuristic for livethrough and earlyclobber registers, allowing us to allocate more inline assembly constructs consistently (rather than the previous allocator doing it by luck).
Tue, Nov 6, 3:10 PM
MatzeB added a comment to D52010: RegAllocFast: Rewrite and improve.

It seems to have a bug now where the first time a physical register is used, it immediately sets kill on it even though there are further uses of the same def

Tue, Nov 6, 3:05 PM

Mon, Nov 5

MatzeB added a comment to D54137: AArch64: Fix invalid CCMP emission.

hmm this is still buggy...

Mon, Nov 5, 7:39 PM
MatzeB added a comment to D54137: AArch64: Fix invalid CCMP emission.

Putting this up for early review, still need to create a minimal testcase.

Mon, Nov 5, 7:22 PM
MatzeB created D54137: AArch64: Fix invalid CCMP emission.
Mon, Nov 5, 7:22 PM
MatzeB added a comment to D54119: [X86] Avoid creating a critical edge during cmov expansion.

Unfortunately, it doesn't seem to fix the problem reported in that PR. But maybe it gets us closer to being able to fix it?

Mon, Nov 5, 12:11 PM

Thu, Nov 1

MatzeB added inline comments to D52878: [test-suite] Add flags for stdthreadbug.cpp when building static.
Thu, Nov 1, 6:17 PM
MatzeB added inline comments to D52878: [test-suite] Add flags for stdthreadbug.cpp when building static.
Thu, Nov 1, 6:12 PM
MatzeB added a comment to D52878: [test-suite] Add flags for stdthreadbug.cpp when building static.

I'm not opposed to this change. But out of curiosity:

Thu, Nov 1, 6:11 PM
MatzeB added a comment to D49671: [SchedModel] Propagate read advance cycles to implicit operands outside instruction descriptor.

This patch is causing some problems in my out-of-tree back-end. We add some MachineOperands on the fly for some uses/defs that are conditional or depend on some circumstances, like how registers were allocated, or which depth a loop is at in a loop nest. With this patch, these manually added operands don't work as we intend.

Would you be in a position to mark your extra operands as explicit operands since the machine does appear to be reading/writing them in your case? (Maybe you have to mark the instruction as variadic, or I would be open to invent a new MCInstrDesc flag if that helps...)

Thu, Nov 1, 6:04 PM
MatzeB added a comment to D49671: [SchedModel] Propagate read advance cycles to implicit operands outside instruction descriptor.

I'm wondering if we could maybe keep the old flexible way to look at MachineOperands and put the functionality which sets the latency to zero in the getOperandLatency hook instead?

@materi: I think your ideas make sense. If you have a patch, could you post it, please?

What I meant was to have subtargets override the default latency calculation if they want to ignore operands that are not in the MCInstrDesc. That is, implement SytemZ::getOperandLatency() and have it return 0 in the cases where DefIdx or UseIdx is too large.

I don't have a patch for this and I really don't know which targets want the new behavior.

My first idea for this patch was to loop over the operands / read advances of the instruction in order to propagate read advances to the non-tablegen super-reg operands (see earlier patch proposal under 'History'). This is more arduous than the simply clearing those latencies during DAG construction, per what was committed. I think however this should work for you as well, or?

I wonder if it would be enough to make a rule to clear latencies only on *implicit* extra operands, and not on explicit ones? In other words if added *explicit* operands were left alone, this would not break anything? But I am not sure if this is possible with variadic instructions, or if it's a good idea...

I don't like any implementation that has first-class and second-class MachineOperands. At least I think it's a bad idea to have this in the default implementation, doing it in a target hook makes sense though.

Thu, Nov 1, 5:59 PM

Wed, Oct 31

MatzeB accepted D53815: [TableGen] Better error checking for TIED_TO constraints..

LGTM. Looks sensible to me, nitpicks below.

Wed, Oct 31, 5:35 PM
MatzeB accepted D53950: Fix clang -Wimplicit-fallthrough warnings across llvm, NFC.

LGTM, thanks for pushing this! Some of the changes here look like things that were just waiting to become an actual problem.

Wed, Oct 31, 5:18 PM

Tue, Oct 30

MatzeB updated the diff for D53885: MachineModuleInfo: Initialize DbgInfoAvailable depending on debug_cus existing.
Tue, Oct 30, 5:56 PM
MatzeB updated the diff for D53909: ADT/STLExtras: Introduce llvm::empty(); NFC.
  • Add a simple unittest
  • Go with the adl_begin() == adl_end() implementation for now. We can always switch to more fine grained overloading when we actually deem it necessary for performance. And after all we will probably just replace it with std::empty() anyway once we use C++17 in llvm.
Tue, Oct 30, 4:50 PM
MatzeB added a comment to D53909: ADT/STLExtras: Introduce llvm::empty(); NFC.

(maybe some unit tests?)

Also, what about having only a single implementation:

template <typename Range>
constexpr bool empty(const Range &r) {
  return adl_begin(r) == adl_end(r);
}

Not sure why the standard library doesn't do it this way, but there are probably good reasons... though maybe they don't apply to us/llvm?

Tue, Oct 30, 4:39 PM
MatzeB created D53909: ADT/STLExtras: Introduce llvm::empty(); NFC.
Tue, Oct 30, 4:10 PM
MatzeB created D53903: MachineOperand/MIParser: Do not print debug-use flag, infer it.
Tue, Oct 30, 3:13 PM
MatzeB created D53885: MachineModuleInfo: Initialize DbgInfoAvailable depending on debug_cus existing.
Tue, Oct 30, 1:23 PM

Tue, Oct 23

MatzeB added a comment to D52010: RegAllocFast: Rewrite and improve.

I'm now depending on this for a change I'm working on rather than figuring out how to fix kill flag handling

I'm not sure what exactly you mean by that. But just as a warning: This patch will lead to more kill flags being added than before, but it does not guarantee that every last use has a kill flag (especially vregs live across blocks will still lack kill flags).

Tue, Oct 23, 6:15 PM
MatzeB updated the diff for D52010: RegAllocFast: Rewrite and improve.

Updated version

Tue, Oct 23, 6:13 PM
MatzeB added inline comments to D53181: SelectionDAG: Reuse bigger sized constants in memset expansion..
Tue, Oct 23, 4:20 PM
MatzeB added a comment to D53176: AArch64/TargetTransformInfo: Report div/rem constant immediate costs as TCC_Free.

ping

Tue, Oct 23, 3:29 PM
MatzeB added a comment to D52010: RegAllocFast: Rewrite and improve.
  • I am slowly adatpting all the tests to the changes here; I have a commit fixing ~100 of them but there is still maybe 30-40 to go.
  • I believe I have the first two asserts fixed locally here.
  • The 3rd assert is picking up invalid MIR (a value is used before it is defined), my local version now has a commandline flag that allows us to disable that assert for the couple tests that show invalid MIR (planning to file PRs for the target owners to fix, so we can remove the commandline flag once the last test is fixed).
Tue, Oct 23, 2:17 PM

Oct 22 2018

MatzeB added a comment to D53485: [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN parent.

The code in PEI::replaceFrameIndices also seems sketchy to me as in case of nested callframes it would set InsideCallSequence to false after the inner ADJCALLSTACKUP (though at a first glance I wasn't sure yet why that variable exists at all, and why we would not track sp adjustments outside of callframe setups...)

Oct 22 2018, 8:37 PM
MatzeB added a comment to D53485: [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN parent.

What about MachineVerifier::verifyStackFrame()? That currently rejects any function with nested callframe setups. You will probably see your testcases fail when running with -verify-machineinstrs.

Oct 22 2018, 8:21 PM
MatzeB accepted D53030: [MicroBenchmark] Add initial LoopInterchange test/benchmark..

Please try whether using DoNotOptimize as suggested works. With that addressed LGTM.

Oct 22 2018, 12:55 PM

Oct 21 2018

MatzeB added a comment to D53424: Enable thread specific cl::opt values for multi-threaded support.

Can you explain what you would use per-thread command line options for?
Intuitively I would not expect actual commandline users wanting to set options per thread. If you need it to tweak compiler behavior then it might be better to find ways to encode the information in TargetOptions.h, function attributes or similar, so we have a streamlined way of setting them independently of programmatically modifying commandline options.

Oct 21 2018, 2:04 PM

Oct 16 2018

MatzeB added inline comments to D53030: [MicroBenchmark] Add initial LoopInterchange test/benchmark..
Oct 16 2018, 4:21 PM
MatzeB added inline comments to D53030: [MicroBenchmark] Add initial LoopInterchange test/benchmark..
Oct 16 2018, 4:17 PM
MatzeB added a comment to D53030: [MicroBenchmark] Add initial LoopInterchange test/benchmark..

Did you make sure this typically finished in 0.5-1s of time?

Oct 16 2018, 4:16 PM

Oct 15 2018

MatzeB accepted D36224: [TwoAddressInstructionPass] Replace subregister uses when processing tied operands.

LGTM

Oct 15 2018, 12:02 AM

Oct 11 2018

MatzeB added inline comments to D53181: SelectionDAG: Reuse bigger sized constants in memset expansion..
Oct 11 2018, 4:22 PM
MatzeB updated the diff for D53181: SelectionDAG: Reuse bigger sized constants in memset expansion..
  • drop accidental reformatting of existing code from the patch.
Oct 11 2018, 4:20 PM
MatzeB updated the summary of D53181: SelectionDAG: Reuse bigger sized constants in memset expansion..
Oct 11 2018, 4:03 PM
MatzeB created D53181: SelectionDAG: Reuse bigger sized constants in memset expansion..
Oct 11 2018, 4:03 PM
MatzeB updated the diff for D53174: X86/TargetTransformInfo: Report div/rem constant immediate costs as TCC_Free.

noticed that the test needs to operate on i64 to have any effect (as all i32 values are reported as TCC_Free anyway)

Oct 11 2018, 3:49 PM
MatzeB added a comment to D53174: X86/TargetTransformInfo: Report div/rem constant immediate costs as TCC_Free.

Or is moving them because its a loop and we don't want to reload it every iteration and don't trust LICM?

I'm not completely sure. I see the code operating with block frequencies so it probably sees a lower block frequency in front of the loop.

The pass itself currently runs relatively close to SelectionDAG so there is no LICM happening anymore. Though IMO it really should be implemented in SelectionDAG after everything is expanded and actually all constants are visible and not play all the games with hoisting constants between blocks.

Oct 11 2018, 3:36 PM
MatzeB added a comment to D53174: X86/TargetTransformInfo: Report div/rem constant immediate costs as TCC_Free.

Or is moving them because its a loop and we don't want to reload it every iteration and don't trust LICM?

Oct 11 2018, 3:34 PM
MatzeB added a comment to D53176: AArch64/TargetTransformInfo: Report div/rem constant immediate costs as TCC_Free.
  • This is not inspired by some benchmark, but from me needing stable tests in an upcoming change making DAGCombiner slightly less aggressive with OpaqueConstants. It does however seem like the right thing to do in general.
Oct 11 2018, 3:21 PM
MatzeB created D53176: AArch64/TargetTransformInfo: Report div/rem constant immediate costs as TCC_Free.
Oct 11 2018, 3:17 PM
MatzeB added a comment to D53174: X86/TargetTransformInfo: Report div/rem constant immediate costs as TCC_Free.

This is currently not motivated by a benchmark but preparation for an upcoming commit of mine where I don't want ConstantHoisting to screw with dev/rem and mess up testcases.

Oct 11 2018, 3:13 PM
MatzeB created D53174: X86/TargetTransformInfo: Report div/rem constant immediate costs as TCC_Free.
Oct 11 2018, 3:12 PM

Oct 8 2018

MatzeB updated subscribers of D52785: [PseudoSourceValue] New category to represent floating-point status.

As far as I understand MachineMemoryOperands:

Oct 8 2018, 3:04 PM

Oct 5 2018

MatzeB accepted D49671: [SchedModel] Propagate read advance cycles to implicit operands outside instruction descriptor.

Sorry for slow response. LGTM, some nitpicks below but feel free to fix testcases and nitpicks at your own discretion.

Oct 5 2018, 3:00 PM
MatzeB added an edge to rL343874: DwarfDebug: Pick next location in case of missing location at block begin: D52862: DwarfDebug: Pick next location in case of missing location at block begin.
Oct 5 2018, 11:33 AM
MatzeB added 1 commit(s) for D52862: DwarfDebug: Pick next location in case of missing location at block begin: rL343874: DwarfDebug: Pick next location in case of missing location at block begin.
Oct 5 2018, 11:33 AM
MatzeB closed D52862: DwarfDebug: Pick next location in case of missing location at block begin.

Landed in rL343874

Oct 5 2018, 11:32 AM
MatzeB added inline comments to D52862: DwarfDebug: Pick next location in case of missing location at block begin.
Oct 5 2018, 11:32 AM

Oct 4 2018

MatzeB added a comment to D52862: DwarfDebug: Pick next location in case of missing location at block begin.

That explains why I couldn't find those line tables in the executables :)

Oct 4 2018, 12:14 PM
MatzeB added a comment to D52862: DwarfDebug: Pick next location in case of missing location at block begin.

Is it possible that your test harness is removing .dSYMs? .. ah, no, what's happening is that clang declines to add a dsymutil step when it's being used to link .o's into an executable.

Oct 4 2018, 11:39 AM
MatzeB added inline comments to D52862: DwarfDebug: Pick next location in case of missing location at block begin.
Oct 4 2018, 11:26 AM
MatzeB added a comment to D52862: DwarfDebug: Pick next location in case of missing location at block begin.
In D52862#1255543, @vsk wrote:
In D52862#1255529, @vsk wrote:

The core idea and test updates look good to me. I'm curious about the size changes: are you just measuring the size of executables, or of .o's? If it's the former, on Darwin, any size change resulting from this patch might indicate bugs.

I measured executable sizes.

Ah, ok. Well, we know we have more work to do to eliminate codegen differences with/without debug info. We're planning on discussing this at the debug info BoF. (It's not a blocker for this patch.)

The actual instructions are exactly the same, at least in the couple examples I picked.

Oct 4 2018, 11:20 AM
MatzeB added a comment to D52862: DwarfDebug: Pick next location in case of missing location at block begin.
In D52862#1255529, @vsk wrote:

The core idea and test updates look good to me. I'm curious about the size changes: are you just measuring the size of executables, or of .o's? If it's the former, on Darwin, any size change resulting from this patch might indicate bugs.

Oct 4 2018, 11:08 AM

Oct 3 2018

MatzeB added a comment to D52125: X86, AArch64, ARM: Do not attach debug location to spill/reload instructions.

Recommitted as rL343895 now, let's hope it passes the tests this time.

Oct 3 2018, 10:26 PM
MatzeB added a comment to D52862: DwarfDebug: Pick next location in case of missing location at block begin.

Comparing two test-suite builds gives me:

Oct 3 2018, 10:23 PM
MatzeB created D52862: DwarfDebug: Pick next location in case of missing location at block begin.
Oct 3 2018, 10:23 PM

Oct 2 2018

MatzeB added a comment to D52125: X86, AArch64, ARM: Do not attach debug location to spill/reload instructions.
In D52125#1253119, @vsk wrote:

In case someone wants to look at debug info issue:

test case
bin/clang++ -fsanitize=hwaddress --target=aarch64-linux-android -gline-tables-only -O0 1.ii -c

I can reproduce the issue. I think the unexpected locations come from DwargDebug::beginInstruction:

if (!DL) {
  // We have an unspecified location, which might want to be line 0.

Apparently there's a toggle which can switch off this behavior: -mllvm -use-unknown-locations=Disable.

... and trying again with that option set, the line table bloat is gone.

By default line 0 locations are enabled if the instruction is the first inst in a block, or is after a label. That seems reasonable, but in light of this issue it might be worth revisiting.

Oct 2 2018, 2:52 PM
MatzeB updated subscribers of D52125: X86, AArch64, ARM: Do not attach debug location to spill/reload instructions.

+CC dsanders

Oct 2 2018, 11:19 AM
MatzeB updated subscribers of D52125: X86, AArch64, ARM: Do not attach debug location to spill/reload instructions.

Feel free to revert for now, but are you sure it is actually this commit?

Oct 2 2018, 11:19 AM

Oct 1 2018

MatzeB added a reviewer for D52761: AArch64: Fix XSeqPairs/WSeqPairs spilling: olista01.
Oct 1 2018, 7:02 PM
MatzeB created D52761: AArch64: Fix XSeqPairs/WSeqPairs spilling.
Oct 1 2018, 6:59 PM
MatzeB added a comment to D52125: X86, AArch64, ARM: Do not attach debug location to spill/reload instructions.
In D52125#1235612, @vsk wrote:

As Matthias points out, a spill/reload can't unambiguously be associated with a specific instruction.

Note that passing an empty location means that the instruction is described by the previous .loc directive in the stream (if one is present). I think the pedantic thing to do would be to use the special "line 0" location. However, that might bloat the line table and possibly interfere with the way llvm currently identifies prologues.

Oct 1 2018, 11:59 AM
MatzeB accepted D52723: [LNT] Fix fallout caused by YAML files ordering..

LGTM

Oct 1 2018, 9:17 AM

Sep 28 2018

MatzeB created D52681: DAGCombiner: StoreMerging: Fix bad index calculating when adjusting mismatching vector types.
Sep 28 2018, 6:51 PM
MatzeB added a comment to D52553: [PHIElimination] Update the regression test for PR16508.

LGTM

Sep 28 2018, 9:18 AM
MatzeB added a comment to D52558: [PHIElimination] Lower a PHI node with only undef uses as IMPLICIT_DEF.

Minor refactoring/renaming as suggested by Matze.

Sep 28 2018, 9:18 AM

Sep 27 2018

MatzeB accepted D52553: [PHIElimination] Update the regression test for PR16508.

LGTM, thanks! Maybe we can even drop the .ll test now?

Sep 27 2018, 9:58 AM
MatzeB added a comment to D36224: [TwoAddressInstructionPass] Replace subregister uses when processing tied operands.

Note that embarassingly there are still a few passes where we skip machine verification in the default pass pipeline, TwoAddressInstruction being one of them. However if this is a new failure triggered by this particular patch then please fix it!

Sep 27 2018, 9:50 AM
MatzeB added inline comments to D52558: [PHIElimination] Lower a PHI node with only undef uses as IMPLICIT_DEF.
Sep 27 2018, 9:31 AM
MatzeB accepted D52558: [PHIElimination] Lower a PHI node with only undef uses as IMPLICIT_DEF.

The patch on its own makes sense to me, so LGTM (comment below).

Sep 27 2018, 9:27 AM
MatzeB accepted D52612: [Sparc] EXPENSIVE_CHECKS now passes all machine verifier errors (PR27461).

yay \o/ LGTM

Sep 27 2018, 8:28 AM

Sep 24 2018

MatzeB accepted D52410: Use TRI->regsOverlap() in MachineBasicBlock::computeRegisterLiveness.

code change LGTM.
I'd be great if you can find a test for this. (Just in case the AMDGPU based test doesn't work out I'd be fine without one here, given how rarely computeRegisterLiveness() is used)

Sep 24 2018, 5:33 PM
MatzeB accepted D52374: [MachineCopyPropagation] Reimplement CopyTracker in terms of register units.

I'm convinced now the clobbering works correctly because as clobberRegister does not just clobber the register at hand but the whole destination register of the copy touched by the COPY at hand.
LGTM (this time with the right review :)

Sep 24 2018, 5:31 PM
MatzeB added a comment to D51524: [ARM64] [Windows] Handle funclets.

I don't really know how exception handling on windows works or what a funclet is. So here's just a couple style comments and questions

Sep 24 2018, 5:25 PM