MatzeB (Matthias Braun)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 13 2013, 3:10 PM (222 w, 6 d)

Recent Activity

Fri, Nov 17

MatzeB accepted D40204: [test-suite] Fix Xcode SDK cmake cache for benchmarks using re_comp.

LGTM

Fri, Nov 17, 6:13 PM

Thu, Nov 16

MatzeB accepted D40075: [test-suite] Fix unix-smail symbol clash with the symbols in libc when linking statically..

LGTM, thanks.

Thu, Nov 16, 5:20 PM

Wed, Nov 15

MatzeB added a comment to D40075: [test-suite] Fix unix-smail symbol clash with the symbols in libc when linking statically..

I'd be fine with renaming the functions in the benchmark.

Wed, Nov 15, 4:16 PM
MatzeB added a comment to D40075: [test-suite] Fix unix-smail symbol clash with the symbols in libc when linking statically..

Does this work with all linkers? I don't see this document in the manpage of apples ld64.

Wed, Nov 15, 4:15 PM
MatzeB added a comment to D40077: [RFC][LNT][test-suite] - Allow 1 test to report multiple individual test results.

I'm surprised that lit lets you sneak a dictionary into the reported metrics.

Wed, Nov 15, 11:07 AM
MatzeB added a comment to D40061: [WIP] [ARM] Make MachineVerifier more strict about terminators.

Uh, I wasn't aware that if-conversion could produce terminator instructions in the middle of a basic block, and I would suspect this to break other things like MachineBasicBlock::isReturnBlock() which assume terminators are at the end...

Wed, Nov 15, 10:58 AM
MatzeB added a comment to D39788: [MC] Function stack size section..

I'd defer to others to comment on the highlevel whether this is a good or helpful idea. Some comments though:

  • The format of the new section ought to be documented properly in the llvm docs and not just in the commit message and sourcecode.
  • I think this should report a special "unknown" value for functions with dynamic stack allocations.
  • I'm not 100% sure that MachineFrameInfo::getStackSize() captures all possible allocations; it probably corresponds to what is allocated in the prologue), however I wonder if we can have cases where callframes are not allocated as part of the prologue. Probably need a stronger wording on what "StackSize" in MachineFrameInfo means if you want to output it to the user in this form.
Wed, Nov 15, 10:51 AM
MatzeB added a comment to D39788: [MC] Function stack size section..

I'd defer to others to comment on the highlevel whether this is a good or helpful idea. Some comments though:

Wed, Nov 15, 10:43 AM
MatzeB added a comment to D39536: [PowerPC] Eliminate redundant register copys after register allocation.

Is this like the existing MachineCopyPropagation pass but working accross multiple blocks? Would D30751 solve your problem as well?

Wed, Nov 15, 10:32 AM
MatzeB added inline comments to D39400: WIP: [MachineOperand][MIR] Add isRenamable to MachineOperand..
Wed, Nov 15, 10:28 AM
MatzeB accepted D39044: [MachineRegisterInfo] Avoid having dbg.values affect code generation.

LGTM, thanks.

Wed, Nov 15, 9:46 AM
MatzeB accepted D38351: MIScheduler improved handling of copied physregs.

LGTM with nitpicks.

Wed, Nov 15, 9:42 AM

Mon, Nov 13

MatzeB added a comment to rL317647: Target/TargetInstrInfo.h -> CodeGen/TargetInstrInfo.h to match layering.

We should also move TargetCallingConv.h, TargetLowering*.h TargetOpcodes.h, TargetRegisterInfo.h and TargetSubtargetInfo.h. I wonder how we ended up in todays situation...

Mon, Nov 13, 1:28 PM

Mon, Nov 6

MatzeB accepted D39696: [MIRPrinter] Use %subreg.xxx syntax for subregister index operands.

Ah yes, should have done this from the start when introducing the subreg syntax.

Mon, Nov 6, 1:33 PM

Wed, Nov 1

MatzeB added a comment to D39506: Add a script to run various benchmarks and send the result to lnt.

Lnt is both a server and a set of script for benchmarking llvm.

Wed, Nov 1, 1:51 PM

Mon, Oct 30

MatzeB added a comment to D38962: [test-suite] Update bitreverse benchmark..

We also have a live installation at http://lnt.llvm.org
A few of our buildbots submit data there.

Mon, Oct 30, 10:15 AM
MatzeB added a comment to D38962: [test-suite] Update bitreverse benchmark..

While the change itself makes sense:

  • Please give some more descriptions than "update test" in the future.
  • Also give a warning in the commit message as this changes performance and will show up in the LNT numbers.
Mon, Oct 30, 9:59 AM

Thu, Oct 26

MatzeB accepted D39291: [Docs] Use `db_default` instead of `default` in example submit url. .

LGTM

Thu, Oct 26, 7:16 PM

Tue, Oct 24

MatzeB added a comment to D39235: [MachineScheduler] minor refactoring: new static function checkResourceLimit().

A clear "meh" from my side :)
Feel free to commit it.

Tue, Oct 24, 9:56 AM
MatzeB accepted D39223: [opt] Initialize WriteBitcode pass..

Seems obvious, LGTM.

Tue, Oct 24, 9:53 AM

Mon, Oct 23

MatzeB accepted D35844: Correct dwarf unwind information in function epilogue.

LGTM, thanks!

Mon, Oct 23, 10:37 PM

Oct 17 2017

MatzeB added a comment to D37467: Add a new pass to speculate around PHI nodes with constant (integer) operands when profitable..

Without having read the code yet:

Oct 17 2017, 2:22 PM
MatzeB added a comment to D38754: Prevent Machine Copy Propagation from replacing live copy with the dead one.

I'm not sure I understand what exactly is going on. dead and kill flags work just as well for subregisters; I don't see a single COPY in your MI excerpt so I'm not sure what is going on or how copy propagation could get it wrong.

Oct 17 2017, 1:58 PM
MatzeB added a comment to D35844: Correct dwarf unwind information in function epilogue.

Is there anything else that should be done for this patch?

Oct 17 2017, 9:38 AM

Oct 16 2017

MatzeB added a comment to D37415: Add gtest include directories before system include directories on FreeBSD and DragonFlyBSD.

Thank you! Since I am new here, what should I do with the bug: https://bugs.llvm.org/show_bug.cgi?id=30877

Oct 16 2017, 8:43 PM
MatzeB added a comment to D37415: Add gtest include directories before system include directories on FreeBSD and DragonFlyBSD.

Note that I took the liberty and changed the commit message to match the new solution.

Oct 16 2017, 4:04 PM
MatzeB added a comment to D37415: Add gtest include directories before system include directories on FreeBSD and DragonFlyBSD.

I must have missed this in the flood of E-Mail. In the future, feel free to ping the review thread when nothing happened for a week.

Oct 16 2017, 4:03 PM

Oct 13 2017

MatzeB accepted D38754: Prevent Machine Copy Propagation from replacing live copy with the dead one.

LGTM, thanks!

Oct 13 2017, 3:51 PM

Oct 12 2017

MatzeB added a comment to D38489: TargetMachine: Merge TargetMachine and LLVMTargetMachine.

Turns out this doesn't work nice with the current layering. There are some tools that just link libTarget but not libCodeGen. Merging LLVMTargetMachine and TargetMachine however forces us to move TargetMachine into libCodeGen breaking such users.

Oct 12 2017, 5:26 PM
MatzeB abandoned D38482: TargetMachine: Use LLVMTargetMachine in CodeGen.
Oct 12 2017, 5:23 PM

Oct 11 2017

MatzeB accepted D38735: [ScheduleDAGInstrs] fix behavior of getUnderlyingObjectsForCodeGen when no identifiable object found.

Thanks, LGTM

Oct 11 2017, 10:07 PM
MatzeB requested changes to D38164: [MachineScheduler] Favor instructions that do not increase pressure..
Oct 11 2017, 4:47 PM
MatzeB added inline comments to D38351: MIScheduler improved handling of copied physregs.
Oct 11 2017, 4:35 PM
MatzeB added a comment to D38735: [ScheduleDAGInstrs] fix behavior of getUnderlyingObjectsForCodeGen when no identifiable object found.

Do you mean using boolean return value also in getUnderlyingObjectsForInstr for consistency?

Yes that's what I meant.

Oct 11 2017, 4:06 PM
MatzeB accepted D38406: [dump] Remove NDEBUG from test to enable dump methods [NFC].

Just noticed I didn't get around accepting this even though it looks good. LGTM

Oct 11 2017, 4:05 PM
MatzeB accepted D38616: [RegisterCoalescer] Don't set read-undef in pruneValues, only clear.

LGTM

Oct 11 2017, 1:44 PM

Oct 10 2017

MatzeB added a comment to D38154: [PassManager] Run global opts after the inliner.

Some more points on benchmarking (really should rather write more docu than commenting here...):

  • Use ninja -j1 to get less noisy compiletime measurements (there of course are a lot of other things you can and should do to reduce the noise level on your system, search for apropriate llvm-dev mailinglist posts etc.)
  • On macOS you may need -C ../test-suite/cmake/caches/target-x86_64-macosx.cmake to pick up the right include directories from xcode/xcrun.
  • As with any cmake project I find ccmake . really helpful to discover possible options and flags.
  • To reduce noise, run the process multiple times and let compare.py take the minimum: compare.py base_run0.json base_run1.json base_run2.json vs patched_run0.json patched_run1.json patched_run2.json (Note: the vs is an actual commandline argument)
Oct 10 2017, 3:13 PM
MatzeB added a comment to D38154: [PassManager] Run global opts after the inliner.

For the record, measuring CTMark can be as easy as this without any LNT involved:

Oct 10 2017, 3:07 PM
MatzeB added a comment to D36704: [CodeGen] Improve the consistency of instruction fusion.

You can push this if you want, but I still think that first too loops in fuseInstructionPair() are a poor mans version of the reachability check I outlined in the comments (which would catch more cases).

Oct 10 2017, 2:34 PM
MatzeB added a comment to D38754: Prevent Machine Copy Propagation from replacing live copy with the dead one.

Machine Copy Propagation does not expect dead stuff to appear so far downstairs. So, it mistakenly replaces live copy with upper one that is marked as dead.
This patch is temporary workaround until Rename Disconnected Subregister Components is fixed.

This is first a bug in machine copy propagation, there is simply no rule that dead copies are not allowed. (If you really want to make it a rule then you really should write a MachineVerifier check for it!)
Do you have any experience how common it is for RenameIndependentSubregs to produce dead code? I'd rather keep that code simpler and purely a renaming operation as the name suggests. Do you know whether the situation is already detected by DetectDeadLanes? I'd be more open to add dead code elimination to that pass (as we could run it instead of dead code elimination I believe).

Oct 10 2017, 2:26 PM
MatzeB added a comment to D38616: [RegisterCoalescer] Don't set read-undef in pruneValues, only clear.

Thanks for the patch and the testcase, this looks good.

Oct 10 2017, 12:02 PM
MatzeB added a comment to D38735: [ScheduleDAGInstrs] fix behavior of getUnderlyingObjectsForCodeGen when no identifiable object found.

This looks good, but makes me wonder if we shouldn't do the same change (returning a bool instead of looking at the vector containing 0 or more elements) to getUnderlyingObjectsForInstr() for consistency. Could you try doing that?

Oct 10 2017, 10:14 AM
MatzeB added a comment to D38734: [MachineCombiner] Fix initialisation of LastUpdate for incremental update..

Please take a look at http://llvm.org/docs/MIRLangRef.html#simplifying-mir-files and simplify the test.

Oct 10 2017, 9:23 AM

Oct 5 2017

MatzeB updated subscribers of D38597: [PEI] Remove required properties and use 'if' instead of std::function.
Oct 5 2017, 1:57 PM
MatzeB accepted D38597: [PEI] Remove required properties and use 'if' instead of std::function.

LGTM; maybe wait a bit for @qcolombet to comment.

Oct 5 2017, 1:56 PM
MatzeB added inline comments to D38597: [PEI] Remove required properties and use 'if' instead of std::function.
Oct 5 2017, 1:44 PM
MatzeB added a comment to D38597: [PEI] Remove required properties and use 'if' instead of std::function.

+1. You could still check the condition in an assert() at the right place though.

Oct 5 2017, 1:35 PM
MatzeB added a comment to D35844: Correct dwarf unwind information in function epilogue.

This is starting to look good, a few more nitpicks below and hopefullly @iteratee will comment on the isDirective() thing.

Oct 5 2017, 9:48 AM

Oct 4 2017

MatzeB accepted D38496: [XRay] [test-suite] Add litsupport module for microbenchmarks.

Thanks, LGTM

Oct 4 2017, 11:03 AM
MatzeB accepted D38534: [TablgeGen] : Tidy up CodeGenSchedule. NFC..

Sure feel free to also make such changes in tablegen without review. LGTM

Oct 4 2017, 11:02 AM

Oct 3 2017

MatzeB added reviewers for D38511: [cmake] Add LLVM_ENABLE_DEBUG variable which can be used to replace NDEBUG: chandlerc, echristo, hfinkel.

Thanks for working on this!

Oct 3 2017, 12:54 PM
MatzeB added a comment to D35844: Correct dwarf unwind information in function epilogue.

Thanks @violetav this looks a lot better as a self-contained pass!

Oct 3 2017, 11:50 AM
MatzeB added inline comments to D38496: [XRay] [test-suite] Add litsupport module for microbenchmarks.
Oct 3 2017, 10:54 AM
MatzeB added inline comments to D38496: [XRay] [test-suite] Add litsupport module for microbenchmarks.
Oct 3 2017, 10:30 AM

Oct 2 2017

MatzeB added a comment to D38489: TargetMachine: Merge TargetMachine and LLVMTargetMachine.

Although not necessarily a measure for code quality; here is:

Oct 2 2017, 9:01 PM
MatzeB added a comment to D38482: TargetMachine: Use LLVMTargetMachine in CodeGen.

Merging the two isn't too hard either: D38489

Oct 2 2017, 8:38 PM
MatzeB created D38489: TargetMachine: Merge TargetMachine and LLVMTargetMachine.
Oct 2 2017, 8:35 PM
MatzeB added a comment to D38482: TargetMachine: Use LLVMTargetMachine in CodeGen.

And for what it's worth for some of the methods I moved around here:

  • usesPhysRegsForPEI() this one should be possible to eliminate with some refactoring where we factor out register scavenging into a separate pass from PEI. (That's somewhere on my TODO list not very high up though).
  • useIPRA() this one could just as well be a know in TargetPassConfig.
Oct 2 2017, 7:36 PM
MatzeB added a comment to D38482: TargetMachine: Use LLVMTargetMachine in CodeGen.

Hi Matthias,

Similar to Chandler's comments I think we should look at folding these two together rather than keeping them split apart. I started working a little bit in this direction at one point, but admittedly got distracted by other things. A more concrete thought here is that we could either move all of the bits over into the TargetMachine and for something that doesn't need it then that target could simply just not initialize or define those initialization routines (I don't recall anything that doesn't currently need). It might end up being a bit more verbose in a couple of targets, but seems like a reasonable start.

Thoughts?

Oct 2 2017, 7:34 PM
MatzeB added a comment to D38482: TargetMachine: Use LLVMTargetMachine in CodeGen.

I generally like the direction of the cleanup. A few high level questions:

  1. You say that this is to distinguish targets that *do* use LLVM's lib/CodeGen from those that do not. Do you mean entire targets that do not? Or do you mean just *components* of targets that do not depend on it like the target's MC layer?

Yes I mean entire targets.

Oct 2 2017, 7:31 PM
MatzeB accepted D38182: test-suite: add cpu features detection in configuration.

Tentatively approving this, but please hold on with committing until we have a patch approved that actually uses this.

Oct 2 2017, 5:34 PM
MatzeB added inline comments to D38484: test-suite: add cpu architecture detection in cmake configuration.
Oct 2 2017, 5:32 PM
MatzeB added inline comments to D38484: test-suite: add cpu architecture detection in cmake configuration.
Oct 2 2017, 5:31 PM
MatzeB added a comment to D38484: test-suite: add cpu architecture detection in cmake configuration.

This seems to be all x86 specific, so I think you should have x86 in the names of CPU_ARCH, DetectCPUArchitecture.c and detect_cpu_architecture.

Oct 2 2017, 5:27 PM
MatzeB created D38482: TargetMachine: Use LLVMTargetMachine in CodeGen.
Oct 2 2017, 4:01 PM
MatzeB accepted D38447: [MiSched] - Simplify ProcResEntry access.

LGTM.
Feel free to just push without review if you are just changing code to equivalent range-based for constructs like this in the future.

Oct 2 2017, 10:48 AM

Sep 29 2017

MatzeB added inline comments to D36704: [CodeGen] Improve the consistency of instruction fusion.
Sep 29 2017, 3:29 PM
MatzeB added inline comments to D36704: [CodeGen] Improve the consistency of instruction fusion.
Sep 29 2017, 3:23 PM
MatzeB added inline comments to D36704: [CodeGen] Improve the consistency of instruction fusion.
Sep 29 2017, 1:59 PM
MatzeB added a comment to D38406: [dump] Remove NDEBUG from test to enable dump methods [NFC].

I like this change.

Sep 29 2017, 11:01 AM
MatzeB added a comment to D38253: [ARM] Restore the right frame pointer register in Int_eh_sjlj_longjmp.

FWIW, this was more or less approved by @t.p.northover on IRC:

"The second one is ugly, but that describes SjLj as a whole so I doubt we'd be making the world a worse place." and "There was actually some talk about standardising FP a while ago. Doesn't seem to have gone anywhere yet though."

Sep 29 2017, 8:34 AM

Sep 28 2017

MatzeB added a comment to D38351: MIScheduler improved handling of copied physregs.

Hmm interesting observation. So to recap (and for further comments in the sourcecode :) you seem to deal with this situation:

Sep 28 2017, 6:52 PM
MatzeB updated the diff for D33650: MachineVerifier: Verify liveins list.
  • Rebased on ToT.
  • Now that we have the Restored member on CalleeSavedInfo we can deal with the ARM/LR problems cleanly.
  • Factor out the pristine/reserved register check to a separate commit for now, it's hard enough to get tests passing without them.
  • Various fixes.
Sep 28 2017, 4:08 PM
MatzeB added inline comments to D38306: Cleanup some problems with LLVM_ENABLE_DUMP in release builds..
Sep 28 2017, 10:57 AM
MatzeB added inline comments to D38306: Cleanup some problems with LLVM_ENABLE_DUMP in release builds..
Sep 28 2017, 10:29 AM
MatzeB added inline comments to D38306: Cleanup some problems with LLVM_ENABLE_DUMP in release builds..
Sep 28 2017, 10:27 AM
MatzeB added inline comments to D38306: Cleanup some problems with LLVM_ENABLE_DUMP in release builds..
Sep 28 2017, 10:13 AM

Sep 27 2017

MatzeB added inline comments to D38306: Cleanup some problems with LLVM_ENABLE_DUMP in release builds..
Sep 27 2017, 5:59 PM
MatzeB added inline comments to D38306: Cleanup some problems with LLVM_ENABLE_DUMP in release builds..
Sep 27 2017, 5:57 PM
MatzeB added a comment to D38306: Cleanup some problems with LLVM_ENABLE_DUMP in release builds..

Ideally, I think all the headers would hide the method behind #ifdef LLVM_ENABLE_DUMP , so that it wouldn't be a linker failure but a compile time failure!

Sep 27 2017, 3:06 PM
MatzeB added a comment to D34596: [X86]: Adding a new priority function 'guided-src' for Scheduler DAG instruction scheduling..

Sorry about this review stalling. To give at least some feedback:

Sep 27 2017, 1:02 PM
MatzeB accepted D38306: Cleanup some problems with LLVM_ENABLE_DUMP in release builds..

Thanks, LGTM

Sep 27 2017, 12:53 PM
MatzeB added inline comments to D38306: Cleanup some problems with LLVM_ENABLE_DUMP in release builds..
Sep 27 2017, 11:11 AM
MatzeB added reviewers for D38306: Cleanup some problems with LLVM_ENABLE_DUMP in release builds.: thakis, chandlerc, ahatanak.

Thanks for working on it. Looks fine to me, but I think the cmake logic needs adjustment.

Sep 27 2017, 10:47 AM

Sep 26 2017

MatzeB accepted D37415: Add gtest include directories before system include directories on FreeBSD and DragonFlyBSD.

That looks cleaner and good to me, thanks.

Sep 26 2017, 7:13 PM
MatzeB added a comment to D38267: CMake: Add option to set LLVM_ENABLE_DUMP.

With phabricator mails back, I just realized that I accidentally committed this yesterday before reviews working, sorry.

Sep 26 2017, 3:56 PM
MatzeB closed D35960: [MachineOutliner] Cleanup: move findCandidates out of suffix tree.

Looks like this was committed in r309334

Sep 26 2017, 3:30 PM
MatzeB added a comment to D38182: test-suite: add cpu features detection in configuration.

This looks mostly fine.

Sep 26 2017, 3:30 PM
MatzeB closed D37085: [MachineOutliner] Add missed optimization remarks based off outliner cost model.

Looks like this was committed in r312194

Sep 26 2017, 3:30 PM
MatzeB added inline comments to D38182: test-suite: add cpu features detection in configuration.
Sep 26 2017, 3:30 PM
MatzeB added inline comments to D37891: Driver: hoist the `wchar_t` handling to the driver.
Sep 26 2017, 3:30 PM · Restricted Project
MatzeB closed D30670: [Outliner] Add tail call support.

Looks like this was committed in r297653

Sep 26 2017, 3:30 PM
MatzeB closed D30914: [Outliner] Add outliner for AArch64.

Looks like this was committed in r298162

Sep 26 2017, 3:30 PM
MatzeB closed D23097: RegScavenging: Add scavengeRegisterBackwards().

This landed a while ago in r305625 (as part of D21885)

Sep 26 2017, 3:30 PM
MatzeB accepted D23097: RegScavenging: Add scavengeRegisterBackwards().
Sep 26 2017, 3:30 PM
MatzeB closed D36435: [MachineOutliner] Ensure AArch64 outliner doesn't mess with any register that overlaps with LR.

Looks like this was committed in r310422

Sep 26 2017, 3:30 PM
MatzeB closed D37401: [MIParser] Make sure that getHexUint doesn't produce APInts with a bitwidth of 0.

Looks like this was committed in r312387

Sep 26 2017, 3:30 PM
MatzeB abandoned D27724: Statistic: Initialize in the constructor.

People don't seem to accept static constructors for this.

Sep 26 2017, 3:30 PM
MatzeB accepted D29479: Driver: Do not warn about unused -pthread when linking on darwin.
Sep 26 2017, 3:30 PM
MatzeB closed D29479: Driver: Do not warn about unused -pthread when linking on darwin.

This was accepted on the mailinglist and committed in r294065

Sep 26 2017, 3:30 PM