Page MenuHomePhabricator

mtrofin (Mircea Trofin)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 10 2015, 11:03 PM (302 w, 2 d)

Recent Activity

Yesterday

mtrofin added inline comments to D91358: [ARM][RegAlloc] Add t2LoopEndDec.
Wed, Nov 25, 9:46 AM · Restricted Project

Sat, Nov 21

mtrofin committed rG2482648a795a: thinlto_embed_bitcode.ll: clarify grep should treat input as text (authored by mtrofin).
thinlto_embed_bitcode.ll: clarify grep should treat input as text
Sat, Nov 21, 9:48 PM

Thu, Nov 19

mtrofin added a reviewer for D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner': tejohnson.
Thu, Nov 19, 9:12 PM · Restricted Project, Restricted Project
mtrofin updated the diff for D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'.

patched up tests

Thu, Nov 19, 9:11 PM · Restricted Project, Restricted Project
mtrofin committed rGf62fe0ee3bff: [FileCheck] Disallow unused prefixes in llvm/test/Analysis (authored by mtrofin).
[FileCheck] Disallow unused prefixes in llvm/test/Analysis
Thu, Nov 19, 7:57 AM
mtrofin closed D91275: [FileCheck] Disallow unused prefixes in llvm/test/Analysis.
Thu, Nov 19, 7:57 AM · Restricted Project
mtrofin updated the diff for D91275: [FileCheck] Disallow unused prefixes in llvm/test/Analysis.

Scoped changes to just llvm/test/Analysis

Thu, Nov 19, 7:57 AM · Restricted Project

Wed, Nov 18

mtrofin committed rG8ab2353a4c3d: [NFC][TFUtils] also include output specs lookup logic in loadOutputSpecs (authored by mtrofin).
[NFC][TFUtils] also include output specs lookup logic in loadOutputSpecs
Wed, Nov 18, 9:21 PM
mtrofin closed D91759: [NFC][TFUtils] also include output specs lookup logic in loadOutputSpecs.
Wed, Nov 18, 9:20 PM · Restricted Project
mtrofin requested review of D91759: [NFC][TFUtils] also include output specs lookup logic in loadOutputSpecs.
Wed, Nov 18, 8:56 PM · Restricted Project
mtrofin committed rGb51e844f7a4c: [NFC][TFUtils] Extract out the output spec loader (authored by mtrofin).
[NFC][TFUtils] Extract out the output spec loader
Wed, Nov 18, 8:18 PM
mtrofin closed D91751: [NFC][TFUtils] Extract out the output spec loader.
Wed, Nov 18, 8:18 PM · Restricted Project
mtrofin added inline comments to D91751: [NFC][TFUtils] Extract out the output spec loader.
Wed, Nov 18, 6:11 PM · Restricted Project
mtrofin added inline comments to D91751: [NFC][TFUtils] Extract out the output spec loader.
Wed, Nov 18, 5:50 PM · Restricted Project
mtrofin requested review of D91751: [NFC][TFUtils] Extract out the output spec loader.
Wed, Nov 18, 4:18 PM · Restricted Project
mtrofin added a comment to D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'.

One thing that would be nice would be to have both inliners in the same CGSCC pass manager to avoid doing SCC construction twice, but that would require some shuffling of module/cgscc passes in ModuleInlinerWrapperPass. Maybe as a future cleanup.

There's that benefit to simplifying the module with the always inliner before doing inlining "in earnest" I was pointing earlier at: for the ML policies work, we plan on capturing (sub)graph information. Using the same SCC would not help because the "higher" (callers) parts of the graph would have these mandatory inlinings not completed yet, and thus offer a less accurate picture of the problem space.

Oh I see, caller information is useful.

For compile times: http://llvm-compile-time-tracker.com/?config=O3&stat=instructions&remote=aeubanks.
The previous version of this patch (perf/npmalways) running a couple passes has some small but measurable overhead on some benchmarks, 0.5%.
The version of running everything (perf/npmalways2) hugely increases compile times, almost by 50% in one case.

Wed, Nov 18, 12:14 PM · Restricted Project, Restricted Project
mtrofin added a comment to D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'.

One thing that would be nice would be to have both inliners in the same CGSCC pass manager to avoid doing SCC construction twice, but that would require some shuffling of module/cgscc passes in ModuleInlinerWrapperPass. Maybe as a future cleanup.

Wed, Nov 18, 10:23 AM · Restricted Project, Restricted Project
mtrofin added a comment to D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'.

I'll run this through llvm-compile-time-tracker to see what the compile time implications are.

You mean for the variant where we ran some of the function passes, or you'd try running all of them? Probably the latter would be quite interesting as a 'worst case'.

I was trying the previous patch, but will also try running all function passes, definitely would be interesting.

Wed, Nov 18, 9:50 AM · Restricted Project, Restricted Project
mtrofin added a comment to D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'.

What about removing the existing AlwaysInlinerPass and replacing it with this one? Or is that something you were planning to do in a follow-up change?

That's the plan, yes.

open the opportunity to help the full inliner by performing additional function passes after the mandatory inlinings, but before the full inliner

This change doesn't run the function simplification pipeline between the mandatory and full inliner though, only

if (AttributorRun & AttributorRunOption::CGSCC)
  MainCGPipeline.addPass(AttributorCGSCCPass());

if (PTO.Coroutines)
  MainCGPipeline.addPass(CoroSplitPass(Level != OptimizationLevel::O0));

// Now deduce any function attributes based in the current code.
MainCGPipeline.addPass(PostOrderFunctionAttrsPass());

Right - my point was that we could more easily explore doing so by having this new AlwaysInliner separate. In this patch I included those additional passes because I thought they may be necessary or beneficial, but I can remove them as a first step if they are not necessary. @jdoerfert - should the Attributor be run post-always inlining, or is it fine to not be run?

I'd say start off without running any passes and keeping the status quo (i.e. an NFC patch), then explore adding passes between the inliners in a future patch.

Done

And is there any evidence that running the function simplification pipeline between the mandatory and full inliner is helpful? It could affect compile times.

In the ML-driven -Oz case, we saw some marginal improvement. I haven't in -O3 cases using the "non-ml" inliner. I suspect that it helps in the ML policy case (both -Oz and -O3, on which we're currently working), because: 1) the current -Oz takes some global (module-wide) features, so probably simplifying out the trivial cases helps; and 2) we plan on taking into consideration regions of the call graph, and the intuition is that eliminating the trivial cases (mandatory cases) would rise the visibility (for a training algorithm) of the non-trivial cases.

I'd think that adding the mandatory inliner right before the full inliner in the same CGSCC pass manager would do the job. e.g. add it in ModuleInlinerWrapperPass::ModuleInlinerWrapperPass() right before PM.addPass(InlinerPass());

It would, and what I'm proposing here is equivalent to that, but the proposal here helps with these other explorations, with (arguably) not much of a difference cost-wise in itself (meaning, of course, if we discover there's benefit in running those additional passes, we pay with compile time, but in of itself, factoring the always inliner in its own wrapper, or in the same wrapper as the inliner, doesn't really come at much of a cost).

Now, if we determine there is no value, we can bring it back easily - wdyt?

I'll run this through llvm-compile-time-tracker to see what the compile time implications are.

Wed, Nov 18, 9:49 AM · Restricted Project, Restricted Project
mtrofin updated the diff for D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'.

Running just the always inliner variant, without other passes.

Wed, Nov 18, 9:44 AM · Restricted Project, Restricted Project
mtrofin added inline comments to D91275: [FileCheck] Disallow unused prefixes in llvm/test/Analysis.
Wed, Nov 18, 9:32 AM · Restricted Project
mtrofin added a comment to D91275: [FileCheck] Disallow unused prefixes in llvm/test/Analysis.

gentle reminder - is this OK to land now? Thanks!

Wed, Nov 18, 8:33 AM · Restricted Project

Tue, Nov 17

mtrofin added a comment to D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'.

Performing the mandatory inlinings first simplifies the problem the full inliner needs to solve

That confuses me a bit - is that suggesting that we don't run the AlwaysInliner when we are running the Inliner (ie: we only run the AlwaysInliner at -O0, and use the Inliner at higher optimization levels and let the Inliner do always inlining too)?
& sounds like this is suggesting that would change? That we would now perform always inlining separately from inlining? Maybe that's an orthogonal/separate change from one implementing the always inlining using the Inliner being run in a separate mode?

In the NPM, we didn't run the AlwaysInliner until D86988. See also the discussion there. The normal inliner pass was, and still is, taking care of the mandatory inlinings if it finds them. Of course, if we completely upfronted those (which this patch can do), then the normal inliner wouldn't need to. I'm not suggesting changing that - meaning, it's straightforward for the normal inliner to take care of mandatory and policy-driven inlinings. The idea, though, is that if we upfront the mandatory inlinings, the shape of the call graph the inliner operates over is simpler and the effects of inlining probably more easy to glean by the decision making policy. There are trade-offs, though - we can increase that "ease of gleaning" by performing more function simplification passes between the mandatory inlinings and the full inliner.

OK, so if I understand correctly with the old Pass Manager there were two separate passes (always inliner and inliner - they share some code though, yeah?)

AlwaysInlinerLegacyPass does, yes. The NPM variant doesn't.

The NPM always inliner doesn't share any code with the NPM non-always inliner? (though this ( https://reviews.llvm.org/D86988 ) is the patch that added a separate always inliner to the NPM, right? And that patch doesn't look like it adds a whole new pass implementation - so looks like it's sharing some code with something?)

Tue, Nov 17, 3:26 PM · Restricted Project, Restricted Project
mtrofin added a comment to D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'.

What about removing the existing AlwaysInlinerPass and replacing it with this one? Or is that something you were planning to do in a follow-up change?

Tue, Nov 17, 12:20 PM · Restricted Project, Restricted Project
mtrofin added inline comments to D91275: [FileCheck] Disallow unused prefixes in llvm/test/Analysis.
Tue, Nov 17, 6:56 AM · Restricted Project
mtrofin updated the diff for D91275: [FileCheck] Disallow unused prefixes in llvm/test/Analysis.

Addressed comments

Tue, Nov 17, 6:55 AM · Restricted Project

Mon, Nov 16

mtrofin added a comment to D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'.

Performing the mandatory inlinings first simplifies the problem the full inliner needs to solve

That confuses me a bit - is that suggesting that we don't run the AlwaysInliner when we are running the Inliner (ie: we only run the AlwaysInliner at -O0, and use the Inliner at higher optimization levels and let the Inliner do always inlining too)?
& sounds like this is suggesting that would change? That we would now perform always inlining separately from inlining? Maybe that's an orthogonal/separate change from one implementing the always inlining using the Inliner being run in a separate mode?

In the NPM, we didn't run the AlwaysInliner until D86988. See also the discussion there. The normal inliner pass was, and still is, taking care of the mandatory inlinings if it finds them. Of course, if we completely upfronted those (which this patch can do), then the normal inliner wouldn't need to. I'm not suggesting changing that - meaning, it's straightforward for the normal inliner to take care of mandatory and policy-driven inlinings. The idea, though, is that if we upfront the mandatory inlinings, the shape of the call graph the inliner operates over is simpler and the effects of inlining probably more easy to glean by the decision making policy. There are trade-offs, though - we can increase that "ease of gleaning" by performing more function simplification passes between the mandatory inlinings and the full inliner.

OK, so if I understand correctly with the old Pass Manager there were two separate passes (always inliner and inliner - they share some code though, yeah?)

Mon, Nov 16, 7:55 PM · Restricted Project, Restricted Project
mtrofin added a comment to D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'.

Performing the mandatory inlinings first simplifies the problem the full inliner needs to solve

That confuses me a bit - is that suggesting that we don't run the AlwaysInliner when we are running the Inliner (ie: we only run the AlwaysInliner at -O0, and use the Inliner at higher optimization levels and let the Inliner do always inlining too)?
& sounds like this is suggesting that would change? That we would now perform always inlining separately from inlining? Maybe that's an orthogonal/separate change from one implementing the always inlining using the Inliner being run in a separate mode?

Mon, Nov 16, 4:31 PM · Restricted Project, Restricted Project
mtrofin added a comment to D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'.

Please note: the patch isn't 100% ready, there are those tests that check how the pipeline is composed, which are unpleasant to fix, so I want to defer them to after we get agreement over the larger points this patch brings (i.e. pre-performing always inlinings, value in further exploring cleanups before full inlining, etc)

Mon, Nov 16, 2:27 PM · Restricted Project, Restricted Project
mtrofin requested review of D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'.
Mon, Nov 16, 2:24 PM · Restricted Project, Restricted Project
mtrofin updated the diff for D91275: [FileCheck] Disallow unused prefixes in llvm/test/Analysis.

rebase

Mon, Nov 16, 8:36 AM · Restricted Project

Sun, Nov 15

mtrofin accepted D91496: [CodeGen][X86] Remove unused trivial check-prefixes from all CodeGen/X86 directory..

Thanks for doing this! LGTM, but please wait for signoff from a x86 owner.

Sun, Nov 15, 8:20 AM · Restricted Project

Sat, Nov 14

mtrofin added a comment to D86988: [Inliner] Run always-inliner in inliner-wrapper.

That would be awesome.
One thing is that AlwaysInliner is a required pass (http://llvm.org/docs/WritingAnLLVMNewPMPass.html#required-passes), meaning it'll run even in the case that something like opt-bisect tells it to be skipped. That's because we need to respect alwaysinline semantics. But if we make InlinerPass required, it'll run even when it's not supposed to. If we could make it a separate pass but use the same exact infra but with a different InlineAdvisor(?) that would work for me. So something like factoring out InlinerPass::run.

Ack - let me get together a patch then next week - just wanted to first see if there's a fundamental pushback. Thanks!

https://reviews.llvm.org/D91446 looks like a bug that would be obsolete with your proposal.

Sat, Nov 14, 5:01 PM · Restricted Project

Thu, Nov 12

mtrofin updated the diff for D91275: [FileCheck] Disallow unused prefixes in llvm/test/Analysis.

missing newline

Thu, Nov 12, 3:12 PM · Restricted Project
mtrofin added a comment to D91275: [FileCheck] Disallow unused prefixes in llvm/test/Analysis.

So I'm clear - the final patch is just the lit.local.cfg changes? AFAICT the changes to arith-* are just examples that aren't actually needed

Thu, Nov 12, 8:17 AM · Restricted Project
mtrofin added a comment to D91275: [FileCheck] Disallow unused prefixes in llvm/test/Analysis.

Looks okay, but I'm not really up to speed on the implications of lit substitutions, so it would be better for someone else to review if possible. In the ideal world, we'll revert this and any similar patches and just use the option explicitly, once we've fixed all the tests and changed the default to reduce obfuscation, in my opinion, but I'm not sure it's a big issue.

Thu, Nov 12, 8:14 AM · Restricted Project

Wed, Nov 11

mtrofin added inline comments to D91275: [FileCheck] Disallow unused prefixes in llvm/test/Analysis.
Wed, Nov 11, 10:31 AM · Restricted Project
mtrofin added a reviewer for D91275: [FileCheck] Disallow unused prefixes in llvm/test/Analysis: dblaikie.
Wed, Nov 11, 9:51 AM · Restricted Project
mtrofin added a comment to D91229: [FileCheck] Flip -allow-unused-prefixes to false by default.

because FileCheck won't accept more than one occurrence of --allow-unused-prefixes

Perhaps this is a fixable issue?

Wed, Nov 11, 9:51 AM · Restricted Project, Restricted Project
mtrofin requested review of D91275: [FileCheck] Disallow unused prefixes in llvm/test/Analysis.
Wed, Nov 11, 9:15 AM · Restricted Project
mtrofin added a comment to D91229: [FileCheck] Flip -allow-unused-prefixes to false by default.

Maybe one way to do this is to do what @RKSimon suggests in the D90281, and to enable it on a per-directory basis using lit.local.cfg. That would potentially require changing the "0 or 1" occurrences limitation of the option, in favour of "last one wins" (or first one). Then, you make the change in the lit.local.cfg, flipping the default in each directory as you go. Eventually, once all tests that need it are covered, you can just change the default in FileCheck itself. How does that sound?

Wed, Nov 11, 7:19 AM · Restricted Project, Restricted Project

Tue, Nov 10

mtrofin accepted D91160: [NFC] Use [MC]Register for Hexagon target.

lgtm

Tue, Nov 10, 8:40 PM · Restricted Project
mtrofin added a comment to D91160: [NFC] Use [MC]Register for Hexagon target.

I'd definitely prefer not cleaning up RegisterRef as part of this patch. The register/sub tuple pattern is present throughout the code and needs an independant clean-up. Otherwise we risk this not being a NFC.

Regarding const usage, this will delete the implicit copy constructor. Given the earlier statement that those classes need an independant clean-up, are you okay punting on this?

Tue, Nov 10, 8:39 PM · Restricted Project
mtrofin updated the summary of D91229: [FileCheck] Flip -allow-unused-prefixes to false by default.
Tue, Nov 10, 8:24 PM · Restricted Project, Restricted Project
mtrofin added a comment to D91229: [FileCheck] Flip -allow-unused-prefixes to false by default.

FYI - I realize the change is enormous. I don't necessarily mean to land it as-is, we can chunk it by directories and iteratively update this as those land.

Tue, Nov 10, 8:23 PM · Restricted Project, Restricted Project
mtrofin requested review of D91229: [FileCheck] Flip -allow-unused-prefixes to false by default.
Tue, Nov 10, 8:21 PM · Restricted Project, Restricted Project
mtrofin accepted D90902: [NFC] Use [MC]Register in TwoAddressInstructionPass.

lgtm

Tue, Nov 10, 4:07 PM · Restricted Project
mtrofin added a comment to D90281: [FileCheck] Report missing prefixes when more than one is provided..

Sorry, I misread your original email: you're saying: use lit.local.cfg to
blanket-set the "don't allow unused prefixes" behavior on conforming
directories (I read it backwards).

Tue, Nov 10, 8:04 AM · Restricted Project
mtrofin accepted D91161: [NFC] Use [MC]Register for x86 target.

lgtm

Tue, Nov 10, 7:50 AM · Restricted Project
mtrofin added inline comments to D91161: [NFC] Use [MC]Register for x86 target.
Tue, Nov 10, 7:43 AM · Restricted Project
mtrofin added a comment to D91160: [NFC] Use [MC]Register for Hexagon target.

Maybe the popular RegisterRef can be moved to Register.h? Would that complicate this patch?

Tue, Nov 10, 7:41 AM · Restricted Project
mtrofin updated subscribers of D90281: [FileCheck] Report missing prefixes when more than one is provided..

lit.local.cfg would "blanket-allow" future tests, too. Would that be
desirable?

Tue, Nov 10, 7:32 AM · Restricted Project

Mon, Nov 9

mtrofin committed rG2ac3a7d0c46f: [NFC] Use [MC]Register (authored by mtrofin).
[NFC] Use [MC]Register
Mon, Nov 9, 8:37 AM
mtrofin closed D90795: [NFC] Use [MC]Register.
Mon, Nov 9, 8:37 AM · Restricted Project

Thu, Nov 5

mtrofin committed rG98e8a06e72d4: [FileCheck] Use %ProtectFileCheckOutput in allow-unused-prefixes.txt (authored by mtrofin).
[FileCheck] Use %ProtectFileCheckOutput in allow-unused-prefixes.txt
Thu, Nov 5, 7:08 AM
mtrofin closed D90631: [FileCheck] Use %ProtectFileCheckOutput in allow-unused-prefixes.txt.
Thu, Nov 5, 7:08 AM · Restricted Project
mtrofin added a comment to D90610: [Inline] Fix in handling of ptrtoint in InlineCost.

Thanks for looking into this!

Should places where we use ConstantOffsetPtrs to determine if instructions may be simplified, like CallAnalyzer::visitCmpInst, be also adjusted? Or does the ContantExpr::getICmp call take care of different-sized constant ints?

I don't know. When I debugged PR49890 I just realized that the size mismatch was introduced with the ptrtoint, so I tried doing something about that instead of trying to handle it at every use of ConstantOffsetPtrs. And then I realized that fix seems to have fixed PR38500 as well.

Thu, Nov 5, 7:05 AM · Restricted Project

Wed, Nov 4

mtrofin updated the diff for D90631: [FileCheck] Use %ProtectFileCheckOutput in allow-unused-prefixes.txt.

fix

Wed, Nov 4, 5:57 PM · Restricted Project
mtrofin added a comment to D90631: [FileCheck] Use %ProtectFileCheckOutput in allow-unused-prefixes.txt.

Sorry for the delay - ptal. Thanks!

Wed, Nov 4, 5:57 PM · Restricted Project
mtrofin updated the diff for D90631: [FileCheck] Use %ProtectFileCheckOutput in allow-unused-prefixes.txt.

feedback

Wed, Nov 4, 5:57 PM · Restricted Project
mtrofin requested review of D90795: [NFC] Use [MC]Register.
Wed, Nov 4, 1:28 PM · Restricted Project
mtrofin committed rG5dc47541f9ea: [NFC] Use Register/MCRegister (authored by mtrofin).
[NFC] Use Register/MCRegister
Wed, Nov 4, 12:20 PM
mtrofin closed D90724: [NFC] Use Register/MCRegister.
Wed, Nov 4, 12:20 PM · Restricted Project
mtrofin added inline comments to D90724: [NFC] Use Register/MCRegister.
Wed, Nov 4, 9:08 AM · Restricted Project

Tue, Nov 3

mtrofin added a comment to D90539: Make CallInst::updateProfWeight emit i32 weights instead of i64.

Ideally they'd be i64, see https://reviews.llvm.org/D88609 where I tried doing that, but it was reverted multiple times due to uint64_t overflows and I gave up trying to fix the various issues. There are multiple instances of using uint64_t to do uint32_t arithmetic and checking for overflows, and I eventually got annoyed with all those and gave up.

Tue, Nov 3, 7:29 PM · Restricted Project
mtrofin added a comment to D90539: Make CallInst::updateProfWeight emit i32 weights instead of i64.

Would we rather want them to be i64, or is there a fundamental reason they should be i32?

Tue, Nov 3, 6:40 PM · Restricted Project
mtrofin accepted D90725: [NFC] Use [MC]Register in register allocation.

lgtm

Tue, Nov 3, 3:57 PM · Restricted Project
mtrofin added inline comments to D90724: [NFC] Use Register/MCRegister.
Tue, Nov 3, 3:25 PM · Restricted Project
mtrofin requested review of D90724: [NFC] Use Register/MCRegister.
Tue, Nov 3, 3:22 PM · Restricted Project
mtrofin committed rG34b0a99cce81: [Docs][FileCheck] Small fix. (authored by mtrofin).
[Docs][FileCheck] Small fix.
Tue, Nov 3, 7:10 AM

Mon, Nov 2

mtrofin added inline comments to D90578: [llvm] New worker for ThinLTO whole program devirtualization.
Mon, Nov 2, 1:58 PM
mtrofin committed rG22113341d747: [FileCheck] Added documentation for --allow-unused-prefixes (authored by mtrofin).
[FileCheck] Added documentation for --allow-unused-prefixes
Mon, Nov 2, 12:16 PM
mtrofin closed D90621: [FileCheck] Added documentation for --allow-unused-prefixes.
Mon, Nov 2, 12:16 PM · Restricted Project
mtrofin committed rG61e8a4465597: [NFC][regalloc] Use MCRegister appropriately (authored by mtrofin).
[NFC][regalloc] Use MCRegister appropriately
Mon, Nov 2, 11:49 AM
mtrofin closed D90506: [NFC][regalloc] Use MCRegister appropriately.
Mon, Nov 2, 11:49 AM · Restricted Project
mtrofin accepted D90578: [llvm] New worker for ThinLTO whole program devirtualization.

lgtm

Mon, Nov 2, 11:48 AM
mtrofin accepted D90611: [NFC] Use [MC]Register in Live-ness tracking.

LGTM

Mon, Nov 2, 11:45 AM · Restricted Project
mtrofin added inline comments to D90631: [FileCheck] Use %ProtectFileCheckOutput in allow-unused-prefixes.txt.
Mon, Nov 2, 10:51 AM · Restricted Project
mtrofin added a comment to D90631: [FileCheck] Use %ProtectFileCheckOutput in allow-unused-prefixes.txt.

I think only the FileCheck commands that are themselves FileChecked need the protection. The other ones are intended to be rendered to users (humans running the tests) so would benefit from being able to be customized with FILECHECK_OPTS, I think?

Mon, Nov 2, 10:41 AM · Restricted Project
mtrofin updated the diff for D90631: [FileCheck] Use %ProtectFileCheckOutput in allow-unused-prefixes.txt.

feedback

Mon, Nov 2, 10:33 AM · Restricted Project
mtrofin updated the diff for D90621: [FileCheck] Added documentation for --allow-unused-prefixes.

feedback

Mon, Nov 2, 10:31 AM · Restricted Project
mtrofin added inline comments to D90281: [FileCheck] Report missing prefixes when more than one is provided..
Mon, Nov 2, 10:28 AM · Restricted Project
mtrofin requested review of D90631: [FileCheck] Use %ProtectFileCheckOutput in allow-unused-prefixes.txt.
Mon, Nov 2, 10:27 AM · Restricted Project
mtrofin added inline comments to D90281: [FileCheck] Report missing prefixes when more than one is provided..
Mon, Nov 2, 10:20 AM · Restricted Project
mtrofin added a comment to D90281: [FileCheck] Report missing prefixes when more than one is provided..

The documentation wasn't updated. Please do that.

Mon, Nov 2, 8:54 AM · Restricted Project
mtrofin requested review of D90621: [FileCheck] Added documentation for --allow-unused-prefixes.
Mon, Nov 2, 8:54 AM · Restricted Project
mtrofin committed rGa1aaa9ef783a: [FileCheck] Fix comments and eof in allow-unused-prefixes.txt (authored by mtrofin).
[FileCheck] Fix comments and eof in allow-unused-prefixes.txt
Mon, Nov 2, 8:51 AM
mtrofin added a comment to D90610: [Inline] Fix in handling of ptrtoint in InlineCost.

Thanks for looking into this!

Mon, Nov 2, 8:25 AM · Restricted Project
mtrofin added reviewers for D90610: [Inline] Fix in handling of ptrtoint in InlineCost: kazu, davidxl, eraman.
Mon, Nov 2, 8:12 AM · Restricted Project

Fri, Oct 30

mtrofin requested review of D90506: [NFC][regalloc] Use MCRegister appropriately.
Fri, Oct 30, 3:20 PM · Restricted Project
mtrofin added a comment to D90281: [FileCheck] Report missing prefixes when more than one is provided..

Sorry, got a bit eager submitting, as I wanted to unblock uses of the new flag. Happy to address post-submit feedback, if any!

Fri, Oct 30, 2:22 PM · Restricted Project
mtrofin added a comment to D90496: [FileCheck] Address unused prefixes in tests.

Sorry, hit push very quickly - happy to address post-submit feedback.

Fri, Oct 30, 2:17 PM · Restricted Project
mtrofin committed rGee703eb4726b: [FileCheck] Address unused prefixes in tests (authored by mtrofin).
[FileCheck] Address unused prefixes in tests
Fri, Oct 30, 2:14 PM
mtrofin closed D90496: [FileCheck] Address unused prefixes in tests.
Fri, Oct 30, 2:14 PM · Restricted Project
mtrofin requested review of D90496: [FileCheck] Address unused prefixes in tests.
Fri, Oct 30, 1:56 PM · Restricted Project
mtrofin accepted D90430: [clang][NFC] Remove unused FileCheck prefix.

lgtm

Fri, Oct 30, 12:45 PM · Restricted Project
mtrofin committed rG871d658c9ceb: [FileCheck] Report missing prefixes when more than one is provided. (authored by mtrofin).
[FileCheck] Report missing prefixes when more than one is provided.
Fri, Oct 30, 12:40 PM
mtrofin closed D90281: [FileCheck] Report missing prefixes when more than one is provided..
Fri, Oct 30, 12:39 PM · Restricted Project
mtrofin accepted D90480: [TableGen] Remove spurious GISEL prefix from test..

Lgtm

Fri, Oct 30, 11:04 AM · Restricted Project
mtrofin updated the summary of D90281: [FileCheck] Report missing prefixes when more than one is provided..
Fri, Oct 30, 7:02 AM · Restricted Project