Page MenuHomePhabricator

mtrofin (Mircea Trofin)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 10 2015, 11:03 PM (321 w, 6 d)

Recent Activity

Yesterday

mtrofin updated the diff for D98103: [NPM] Do not run function simplification pipeline unnecessarily.

fixups

Mon, Apr 12, 9:52 PM · Restricted Project
mtrofin added a comment to D98103: [NPM] Do not run function simplification pipeline unnecessarily.

I agree with @aeubanks on removing the NFC from the title. This is changing the functionality, even if it's under a disabled flag.

For additional context, with the flag enabled we're seeing performance regressions which indicate optimizations are being missed and picked up by the subsequent runs of the optimization pass.
The purpose of this initial patch is to stage the transition by first introducing the flag, then focus on understanding where the missed optimizations occur so we can fix (for lack of a better word) the NPM pass pipeline.
When the flag is eventually flipped, it's possible the compile-time benefits also disappear due to the introduction of additional optimization passes, but the current pathological cases of high compile times due to repeating (truly unnecessary) optimizations, should remain resolved.

Mon, Apr 12, 9:51 PM · Restricted Project
mtrofin updated the summary of D98103: [NPM] Do not run function simplification pipeline unnecessarily.
Mon, Apr 12, 9:19 PM · Restricted Project
mtrofin added a comment to D98103: [NPM] Do not run function simplification pipeline unnecessarily.

Do you have any data on whether there are any binary differences for a set of programs/benchmarks with this enabled compared to disabled?

There are, in the strict sense of binary diff. I did not try it on benchmarks.

Interesting. But then doesn't this indicate that the passes were not run unnecessarily? Just curious why this happens, e.g. are some changes not reported properly?

Mon, Apr 12, 3:06 PM · Restricted Project
mtrofin added a comment to D98103: [NPM] Do not run function simplification pipeline unnecessarily.

Do you have any data on whether there are any binary differences for a set of programs/benchmarks with this enabled compared to disabled?

Mon, Apr 12, 2:54 PM · Restricted Project
mtrofin added a comment to D98103: [NPM] Do not run function simplification pipeline unnecessarily.

addressed remaining 2 items

Mon, Apr 12, 12:22 PM · Restricted Project
mtrofin updated the diff for D98103: [NPM] Do not run function simplification pipeline unnecessarily.

test

Mon, Apr 12, 12:22 PM · Restricted Project
mtrofin retitled D98103: [NPM] Do not run function simplification pipeline unnecessarily from [NFC][NPM] Do not run function simplification pipeline unnecessarily to [NPM] Do not run function simplification pipeline unnecessarily.
Mon, Apr 12, 12:09 PM · Restricted Project
mtrofin added inline comments to D98103: [NPM] Do not run function simplification pipeline unnecessarily.
Mon, Apr 12, 12:09 PM · Restricted Project
mtrofin updated the diff for D98103: [NPM] Do not run function simplification pipeline unnecessarily.

feedback

Mon, Apr 12, 12:09 PM · Restricted Project
mtrofin added a comment to D98103: [NPM] Do not run function simplification pipeline unnecessarily.

gentle reminder - thanks!

Mon, Apr 12, 9:40 AM · Restricted Project
mtrofin accepted D100249: [Inliner] Propagate SROA analysis through invariant group intrinsics.

The paranoid in me would argue for having a flag enabling this, should there be any regressions, but if we feel this is benign, ok.

Mon, Apr 12, 9:39 AM · Restricted Project

Thu, Apr 8

mtrofin updated the summary of D98103: [NPM] Do not run function simplification pipeline unnecessarily.
Thu, Apr 8, 9:12 AM · Restricted Project
mtrofin retitled D98103: [NPM] Do not run function simplification pipeline unnecessarily from WIP: don't run function simplification pipeline unnecessarily to [NFC][NPM] Do not run function simplification pipeline unnecessarily.
Thu, Apr 8, 9:12 AM · Restricted Project
mtrofin updated the diff for D98103: [NPM] Do not run function simplification pipeline unnecessarily.

updated patch

Thu, Apr 8, 9:05 AM · Restricted Project

Tue, Apr 6

mtrofin requested review of D99992: [mlgo] Skip AOT-compiling a model if a header/object pair is provided.
Tue, Apr 6, 2:09 PM · Restricted Project

Sat, Apr 3

mtrofin committed rGb32e76c6d507: [mlgo] fix build rules (authored by mtrofin).
[mlgo] fix build rules
Sat, Apr 3, 12:59 PM
mtrofin closed D99829: [mlgo] fix build rules.
Sat, Apr 3, 12:59 PM · Restricted Project
mtrofin added a comment to D99829: [mlgo] fix build rules.

I'll land it to unblock the ml build bots, and because the change should poise no risk elsewhere, but let me know if this could be expressed differently, and I'll follow up.

Sat, Apr 3, 12:43 PM · Restricted Project

Fri, Apr 2

mtrofin requested review of D99829: [mlgo] fix build rules.
Fri, Apr 2, 11:53 PM · Restricted Project

Thu, Apr 1

mtrofin committed rGce61def529e2: [regalloc] Ensure Query::collectInterferringVregs is called before interval… (authored by mtrofin).
[regalloc] Ensure Query::collectInterferringVregs is called before interval…
Thu, Apr 1, 8:33 AM
mtrofin closed D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.
Thu, Apr 1, 8:33 AM · Restricted Project
mtrofin added a comment to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

@xbolva00 - is there anything you'd like to see done to this patch?

Thu, Apr 1, 7:54 AM · Restricted Project
mtrofin added a comment to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

@aditya_nandakumar @qcolombet any idea when a better fix for that issue might be available?

Given Aditya did the prototype I let him comment on that, but I don't expect we land something anytime soon (i.e., at least a couple of months).

That sounds like a reasonable estimate of when I can get it upstreamed.

Thu, Apr 1, 7:45 AM · Restricted Project

Wed, Mar 31

mtrofin added inline comments to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.
Wed, Mar 31, 8:42 AM · Restricted Project
mtrofin updated the diff for D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

feedback

Wed, Mar 31, 8:42 AM · Restricted Project
mtrofin added a comment to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

Quick update on the code quality side, I ran spec2006, the llvm benchmarks, the eigen benchmarks, and a few others, on x86 (FDO, thinlto) with this patch, and with/without enabling consider-local-interval-cost. No significant real effect.

I would expect any large out of order cpu to chew through register mov's like they aren't even there. They just end up as register renames. It will be in-order "little" cores where this hurts the most.

Right, unfortunately I don't have access to those.

Wed, Mar 31, 7:39 AM · Restricted Project
mtrofin added a comment to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

Yeah, I agree. 5% is too much to pay. https://reviews.llvm.org/D69437 measured this option as a 25% speed increase in a something that was important enough to fix, with a 0.1-0.2% compile time effect. That's a very different question.

(What really would like to see - is it would be great if there was someone in the llvm community who cared about O2. Trying to get the best performance for a low compile time, and optimizing that tradeoff. That can do a lot of benefit for a lot of people. This particular issue I have less interest in, It's just a shame to see the compile get things so embarrassingly wrong!)

But this patch has been in tree a long time. I believe best practice is to fix things as-is, not revert and attempt to re-apply after so long.

Quick update on the code quality side, I ran spec2006, the llvm benchmarks, the eigen benchmarks, and a few others, on x86 (FDO, thinlto) with this patch, and with/without enabling consider-local-interval-cost. No significant real effect.

About how to proceed, to add to the "revert original vs not" aspect, the additional trouble is that the original patch has some changes to weight calculation that aren't controlled by neither consider-local-interval-cost, nor by enableAdvancedRASplitCost().

Any pushback, then, to proceeding as planned (fixing the code, but disabling it by default everywhere)? If anything "breaks", there's a quick unblocker for those affected, and meanwhile there's time to investigate the alternatives others mentioned here.

If it's not going to be enabled *anywhere*, even on the AArch64, it sounds like dead code to me, which shouldn't be there.

Not enabled by default. This would still give folks a chance to unblock if they hit regressions by using the flag. We can then remove the portion that's flag controlled when we have the alternative others were mention, or when we feel that, since no one reported regressions, it's safe to remove.

I understand that. The question i'm asking is whether it's useful to have that flag in the first place?
Do we know that people will actually use it?

Wed, Mar 31, 7:32 AM · Restricted Project
mtrofin added a comment to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

Yeah, I agree. 5% is too much to pay. https://reviews.llvm.org/D69437 measured this option as a 25% speed increase in a something that was important enough to fix, with a 0.1-0.2% compile time effect. That's a very different question.

(What really would like to see - is it would be great if there was someone in the llvm community who cared about O2. Trying to get the best performance for a low compile time, and optimizing that tradeoff. That can do a lot of benefit for a lot of people. This particular issue I have less interest in, It's just a shame to see the compile get things so embarrassingly wrong!)

But this patch has been in tree a long time. I believe best practice is to fix things as-is, not revert and attempt to re-apply after so long.

Quick update on the code quality side, I ran spec2006, the llvm benchmarks, the eigen benchmarks, and a few others, on x86 (FDO, thinlto) with this patch, and with/without enabling consider-local-interval-cost. No significant real effect.

About how to proceed, to add to the "revert original vs not" aspect, the additional trouble is that the original patch has some changes to weight calculation that aren't controlled by neither consider-local-interval-cost, nor by enableAdvancedRASplitCost().

Any pushback, then, to proceeding as planned (fixing the code, but disabling it by default everywhere)? If anything "breaks", there's a quick unblocker for those affected, and meanwhile there's time to investigate the alternatives others mentioned here.

If it's not going to be enabled *anywhere*, even on the AArch64, it sounds like dead code to me, which shouldn't be there.

Wed, Mar 31, 7:15 AM · Restricted Project

Tue, Mar 30

mtrofin added a comment to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

Yeah, I agree. 5% is too much to pay. https://reviews.llvm.org/D69437 measured this option as a 25% speed increase in a something that was important enough to fix, with a 0.1-0.2% compile time effect. That's a very different question.

(What really would like to see - is it would be great if there was someone in the llvm community who cared about O2. Trying to get the best performance for a low compile time, and optimizing that tradeoff. That can do a lot of benefit for a lot of people. This particular issue I have less interest in, It's just a shame to see the compile get things so embarrassingly wrong!)

But this patch has been in tree a long time. I believe best practice is to fix things as-is, not revert and attempt to re-apply after so long.

Tue, Mar 30, 6:20 PM · Restricted Project

Sun, Mar 28

mtrofin added a comment to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

Extra compile time is fine for -O3 if it improves a runtime performance of various benchmarks.

3-5% extra compile time for almost no visible perf gain is bad trade off and not only Rust folks would be disapointed.

Sun, Mar 28, 1:54 PM · Restricted Project

Thu, Mar 25

mtrofin committed rG20ad206b6055: [NFC] Module::getInstructionCount() is const (authored by mtrofin).
[NFC] Module::getInstructionCount() is const
Thu, Mar 25, 12:29 PM

Wed, Mar 24

mtrofin added a comment to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

Gentle reminder - I disabled the feature by default on x86 (assuming we're OK with the staged approach).

Wed, Mar 24, 9:12 AM · Restricted Project

Mon, Mar 22

mtrofin updated the diff for D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

Disabling 'advancedRASplitCost' on x86.

Mon, Mar 22, 8:57 PM · Restricted Project
mtrofin added a reviewer for D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration: sanwou01.
Mon, Mar 22, 8:55 PM · Restricted Project
mtrofin updated the diff for D98103: [NPM] Do not run function simplification pipeline unnecessarily.

added a test

Mon, Mar 22, 6:01 PM · Restricted Project

Thu, Mar 18

mtrofin added a reverting change for rG11b70b9e3a74: Revert "[NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable…: rG92ccc6cb17a4: Reapply "[NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable….
Thu, Mar 18, 9:45 AM
mtrofin committed rG92ccc6cb17a4: Reapply "[NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable… (authored by mtrofin).
Reapply "[NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable…
Thu, Mar 18, 9:45 AM
mtrofin committed rG4b1c8070bb8c: [NFC][ArgumentPromotion] Clear FAM cached results of erased function. (authored by mtrofin).
[NFC][ArgumentPromotion] Clear FAM cached results of erased function.
Thu, Mar 18, 9:18 AM

Wed, Mar 17

mtrofin added a comment to rG11b70b9e3a74: Revert "[NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable….

AssumptionCache::invalidate seems wrong. It is immutable for the function it refers to, but not immutable for other functions.

AssumptionCache is designed to be self-updating.
It turns out the problem is that ArgumentPromotion deletes functions without deleting their analyses. New functions could be allocated at the same address as a previously deleted function. The 'cached result' for AssumptionCache was that of a defunct function, in this case.

The previous behavior - clearing - wasn't necessarily guaranteeing this wouldn't happen either (I think), but just harder to hit.

I think a more thorough fix is required here. Perhaps something that requires, from the API, managing a defunct's function's (or other IR) analyses? Not sure yet, and wonder what others' ideas may be.

AssumptionCache::invalidate seems wrong. It is immutable for the function it refers to, but not immutable for other functions.

AssumptionCache is designed to be self-updating.

It turns out the problem is that ArgumentPromotion deletes functions without deleting their analyses. New functions could be allocated at the same address as a previously deleted function. The 'cached result' for AssumptionCache was that of a defunct function, in this case.

If by self-updating, clients should handle it explicitly. ArgumentPromotion should call AssumptionCache::clear() just before the function is deleted.

Wed, Mar 17, 11:17 PM

Tue, Mar 16

mtrofin added a comment to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

@mtrofin I'm not really concerned about regressions on individual test cases, but the average impact. I found https://github.com/llvm/llvm-project/commit/f649f24d388c745d20fab5573d27b822b92818ed with some data from when the option was enabled to AArch64, and it looks like this option has essentially zero average impact and only improves a few rare outliers. This was totally fine at the time, because enabling the option was considered to be essentially free compile-time wise. But now it turns out that this is actually not the case, and the small compile-time impact is actually the result of an implementation bug. The change would have never landed at the time if this fact had been known. A zero geomean improvement on run-time is a terrible trade-off for a 2-3% geomean regression on compile-time.

Unless the situation on X86 is significantly different than for AArch64, it seems pretty clear to me that we should just disable the option until it can be implemented in a way that is both correct and sufficiently cheap.

Tue, Mar 16, 6:45 PM · Restricted Project
mtrofin added a comment to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

Unfortunately I'm not familiar with RegAlloc, but I somehow doubt that this kind of compile-time hit is intrinsically necessary to address this issue.

The problem is that the previous use of the APIs was incorrect - the code before was sometimes using stale data, because it was incorrectly not calling collectInterferingVRegs. The compile time regression is reflective of the effort required to refresh the data.

I think we have a situation like this:

  • (status quo) no compile time regression, but incorrect functionality
  • (patch) compile time regression, correct functionality
  • remove patch that originally introduced this functionality (https://reviews.llvm.org/D35816), but then there are cases where we hit regressions in code quality
  • <options that I am not aware of - actionable suggestions very welcome!>

The best option would be to implement this without the compile-time impact or with low impact -- it's probably possible, but may require a larger time investment. If you don't have the resources for that, then please evaluate the third option. On the assumption that disabling this code only makes for a minor code quality regression, that would be preferred over a large compile-time regression. Of course, if disabling this makes for a large code quality regression, this becomes harder to answer.

Tue, Mar 16, 3:28 PM · Restricted Project
mtrofin added a comment to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

Unfortunately I'm not familiar with RegAlloc, but I somehow doubt that this kind of compile-time hit is intrinsically necessary to address this issue.

The problem is that the previous use of the APIs was incorrect - the code before was sometimes using stale data, because it was incorrectly not calling collectInterferingVRegs. The compile time regression is reflective of the effort required to refresh the data.

Tue, Mar 16, 2:29 PM · Restricted Project
mtrofin added a comment to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

I've reverted this change because it causes significant compile-time regressions, e.g. >5% on sqlite: https://llvm-compile-time-tracker.com/compare.php?from=0aa637b2037d882ddf7861284169abf63f524677&to=d40b4911bd9aca0573752e065f29ddd9aff280e1&stat=instructions I'm assuming a regression of that size wasn't intentional here.

It may be an effect of the code actually performing correctly (see the CL description). I'll confirm that is the case.

Tue, Mar 16, 2:00 PM · Restricted Project
mtrofin added a comment to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

I've reverted this change because it causes significant compile-time regressions, e.g. >5% on sqlite: https://llvm-compile-time-tracker.com/compare.php?from=0aa637b2037d882ddf7861284169abf63f524677&to=d40b4911bd9aca0573752e065f29ddd9aff280e1&stat=instructions I'm assuming a regression of that size wasn't intentional here.

Tue, Mar 16, 12:54 PM · Restricted Project
mtrofin committed rGd40b4911bd9a: [regalloc] Ensure Query::collectInterferringVregs is called before interval… (authored by mtrofin).
[regalloc] Ensure Query::collectInterferringVregs is called before interval…
Tue, Mar 16, 12:19 PM
mtrofin closed D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.
Tue, Mar 16, 12:19 PM · Restricted Project
mtrofin added a comment to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

If there's no pushback, given the previous lgtm (and the subsequent removal of regressions), I'll go ahead with submitting this.

Tue, Mar 16, 8:50 AM · Restricted Project

Mon, Mar 15

mtrofin updated the diff for D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

reworded the comment in LiveRangeMatrix.cpp for more clarity (hopefully)

Mon, Mar 15, 5:29 PM · Restricted Project
mtrofin added inline comments to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.
Mon, Mar 15, 5:09 PM · Restricted Project
mtrofin updated the diff for D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

no regressions

Mon, Mar 15, 5:06 PM · Restricted Project

Sun, Mar 14

mtrofin added inline comments to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.
Sun, Mar 14, 11:37 PM · Restricted Project
mtrofin updated the diff for D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

avoid regression

Sun, Mar 14, 10:11 PM · Restricted Project

Mar 13 2021

mtrofin added a comment to rG11b70b9e3a74: Revert "[NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable….

AssumptionCache::invalidate seems wrong. It is immutable for the function it refers to, but not immutable for other functions.

Mar 13 2021, 11:15 AM

Mar 11 2021

mtrofin added a reverting change for rG5eaeb0fa67e5: [NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function…: rG11b70b9e3a74: Revert "[NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable….
Mar 11 2021, 6:34 PM
mtrofin committed rG11b70b9e3a74: Revert "[NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable… (authored by mtrofin).
Revert "[NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable…
Mar 11 2021, 6:34 PM
mtrofin added a reverting change for D98440: [NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function passes: rG11b70b9e3a74: Revert "[NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable….
Mar 11 2021, 6:34 PM · Restricted Project, Restricted Project
mtrofin committed rG5eaeb0fa67e5: [NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function… (authored by mtrofin).
[NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function…
Mar 11 2021, 6:15 PM
mtrofin closed D98440: [NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function passes.
Mar 11 2021, 6:15 PM · Restricted Project, Restricted Project
mtrofin added a comment to D98440: [NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function passes.

This doesn't break the pipeline tests in llvm/test/Other?
Running check-llvm with expensive checks is probably a good idea to see if there are any weird issues.

Done - nothing else broke, indeed.

Mar 11 2021, 12:54 PM · Restricted Project, Restricted Project
mtrofin updated the diff for D98440: [NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function passes.

feedback

Mar 11 2021, 12:53 PM · Restricted Project, Restricted Project
mtrofin requested review of D98440: [NPM][CGSCC] FunctionAnalysisManagerCGSCCProxy: do not clear immutable function passes.
Mar 11 2021, 10:45 AM · Restricted Project, Restricted Project

Mar 10 2021

mtrofin added inline comments to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.
Mar 10 2021, 12:03 PM · Restricted Project
mtrofin added a comment to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

Gentle reminder - thanks!

Mar 10 2021, 8:42 AM · Restricted Project

Mar 9 2021

mtrofin added a reviewer for D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration: wmi.
Mar 9 2021, 12:10 PM · Restricted Project

Mar 8 2021

mtrofin updated the diff for D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

fixed tests

Mar 8 2021, 9:40 PM · Restricted Project
mtrofin updated the summary of D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.
Mar 8 2021, 9:40 PM · Restricted Project
mtrofin retitled D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration from WIP - don't submit (yet) to [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.
Mar 8 2021, 9:39 PM · Restricted Project
mtrofin requested review of D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.
Mar 8 2021, 8:56 PM · Restricted Project

Mar 5 2021

mtrofin requested review of D98103: [NPM] Do not run function simplification pipeline unnecessarily.
Mar 5 2021, 10:10 PM · Restricted Project

Feb 26 2021

mtrofin committed rG3e992326a510: [NFC][regalloc] const-ed APIs, using MCRegister instead of unsigned (authored by mtrofin).
[NFC][regalloc] const-ed APIs, using MCRegister instead of unsigned
Feb 26 2021, 9:56 AM
mtrofin committed rGa2bfc43ae10e: [NFC] Const-ed 2 APIs in VirtRegMap (authored by mtrofin).
[NFC] Const-ed 2 APIs in VirtRegMap
Feb 26 2021, 9:33 AM
mtrofin committed rGa00f7dc2d539: [NFC] MCRegister fixes in RegisterClassInfo, and const-ed APIs (authored by mtrofin).
[NFC] MCRegister fixes in RegisterClassInfo, and const-ed APIs
Feb 26 2021, 8:58 AM

Feb 20 2021

mtrofin added inline comments to D97054: [MachineVerifier] Confirm that both ends of a tied def/use are live together.
Feb 20 2021, 9:59 PM · Restricted Project

Feb 19 2021

mtrofin added inline comments to D97106: [UpdateTestChecks] Warn if any function conflicts under the same prefix.
Feb 19 2021, 7:34 PM · Restricted Project
mtrofin added a reviewer for D97106: [UpdateTestChecks] Warn if any function conflicts under the same prefix: craig.topper.
Feb 19 2021, 7:28 PM · Restricted Project
mtrofin added a comment to D97106: [UpdateTestChecks] Warn if any function conflicts under the same prefix.

lgtm code-wise. Added folks who were also interested in this, from a user perspective (I'm not a user of the tool, can't comment on that aspect.)

Feb 19 2021, 7:27 PM · Restricted Project
mtrofin added reviewers for D97106: [UpdateTestChecks] Warn if any function conflicts under the same prefix: RKSimon, spatel.
Feb 19 2021, 7:25 PM · Restricted Project
mtrofin committed rG82492f24ffa7: [NFC][Regalloc] Share the VirtRegAuxInfo object with LiveRangeEdit (authored by mtrofin).
[NFC][Regalloc] Share the VirtRegAuxInfo object with LiveRangeEdit
Feb 19 2021, 7:55 AM
mtrofin closed D96898: [NFC][Regalloc] Share the VirtRegAuxInfo object with LiveRangeEdit.
Feb 19 2021, 7:55 AM · Restricted Project

Feb 18 2021

mtrofin added inline comments to D96898: [NFC][Regalloc] Share the VirtRegAuxInfo object with LiveRangeEdit.
Feb 18 2021, 6:55 PM · Restricted Project

Feb 17 2021

mtrofin requested review of D96898: [NFC][Regalloc] Share the VirtRegAuxInfo object with LiveRangeEdit.
Feb 17 2021, 1:45 PM · Restricted Project
mtrofin committed rG3a030c2f2fe3: [NFC][RegAlloc] InlineSpiller::Original is a Register (authored by mtrofin).
[NFC][RegAlloc] InlineSpiller::Original is a Register
Feb 17 2021, 12:08 PM

Feb 16 2021

mtrofin committed rG33481c9997e3: [mlgo] Fetch models from path / URL (authored by mtrofin).
[mlgo] Fetch models from path / URL
Feb 16 2021, 10:48 PM
mtrofin closed D96796: [mlgo] Fetch models from path / URL.
Feb 16 2021, 10:47 PM · Restricted Project
mtrofin updated the diff for D96796: [mlgo] Fetch models from path / URL.

also removed the URL text from help message

Feb 16 2021, 10:35 PM · Restricted Project
mtrofin added a comment to D96796: [mlgo] Fetch models from path / URL.

I'm not sure about the use case for the remote download feature (you can always just curl <URL> | tar xvz). If it's not needed, then I'd skip it for now, we can always introduce it later.

Feb 16 2021, 10:34 PM · Restricted Project
mtrofin added inline comments to D96796: [mlgo] Fetch models from path / URL.
Feb 16 2021, 10:34 PM · Restricted Project
mtrofin updated the diff for D96796: [mlgo] Fetch models from path / URL.

feedback

Feb 16 2021, 10:34 PM · Restricted Project
mtrofin updated the summary of D96796: [mlgo] Fetch models from path / URL.
Feb 16 2021, 10:30 PM · Restricted Project
mtrofin requested review of D96796: [mlgo] Fetch models from path / URL.
Feb 16 2021, 9:10 AM · Restricted Project

Feb 15 2021

mtrofin added a comment to D96290: [tools] UpdateTestPrefix: improved verbosity.

It comes down to us wanting the update scripts to use lists of check-prefixes for every RUN line, allowing us to share prefixes at the function level, typically getting more and more specific down the list. That way we can minimise bloat in the check lines, but still have thorough coverage with many RUNs.

With the move to avoid unused prefixes, there's going to be a lot more cases where the prefix lists need editing every time a test is changed - we need the update scripts to tell us both when a prefix becomes newly unused, but also when a run suddenly isn't checked at all.

Feb 15 2021, 10:50 AM · Restricted Project
mtrofin added a comment to D96290: [tools] UpdateTestPrefix: improved verbosity.

I think this is an improvement from current behavior, so no objections from me, but hopefully someone else can take a look at the implementation.
@RKSimon might have some comments based on:
https://llvm.org/PR49189

I've replied on the bug , but we seem to have lost the original warning as part of D93078

Feb 15 2021, 10:22 AM · Restricted Project
mtrofin committed rG7549524ac541: [NFC] Remove spurious ';' on return line in python code (authored by mtrofin).
[NFC] Remove spurious ';' on return line in python code
Feb 15 2021, 8:42 AM
mtrofin added a comment to D96290: [tools] UpdateTestPrefix: improved verbosity.

gentle reminder

Feb 15 2021, 8:36 AM · Restricted Project

Feb 11 2021

mtrofin added inline comments to D96290: [tools] UpdateTestPrefix: improved verbosity.
Feb 11 2021, 9:50 AM · Restricted Project

Feb 9 2021

mtrofin added inline comments to D96290: [tools] UpdateTestPrefix: improved verbosity.
Feb 9 2021, 2:55 PM · Restricted Project

Feb 8 2021

mtrofin updated the diff for D96290: [tools] UpdateTestPrefix: improved verbosity.

newline

Feb 8 2021, 1:50 PM · Restricted Project
mtrofin requested review of D96290: [tools] UpdateTestPrefix: improved verbosity.
Feb 8 2021, 1:49 PM · Restricted Project
mtrofin added a reverting change for rG87f8a08ce36e: [Utils] Add a switch controlling prefix warnings in UpdateTestChecks: rGf31ea86c808c: Revert "[Utils] Add a switch controlling prefix warnings in UpdateTestChecks".
Feb 8 2021, 11:22 AM