avt77 (Andrew V. Tischenko)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 11 2016, 3:46 AM (118 w, 6 d)

Recent Activity

Fri, Jul 20

avt77 created D49611: [X86] Improved sched models for X86 SHLD/SHRD* instructions.
Fri, Jul 20, 11:39 AM
avt77 committed rL337537: Improved sched model for X86 BSWAP* instrs..
Improved sched model for X86 BSWAP* instrs.
Fri, Jul 20, 2:44 AM
avt77 closed D49477: [X86] Improved sched models for X86 BSWAP* instructions..
Fri, Jul 20, 2:44 AM

Thu, Jul 19

avt77 updated the diff for D49477: [X86] Improved sched models for X86 BSWAP* instructions..

I removed the changes from D48222.

Thu, Jul 19, 5:31 AM
avt77 added a comment to D49477: [X86] Improved sched models for X86 BSWAP* instructions..

Drop the tablegen CodeGenSchedule changes

Thu, Jul 19, 5:24 AM
avt77 updated the diff for D49477: [X86] Improved sched models for X86 BSWAP* instructions..

Comments from Simon and Roman were resolved.

Thu, Jul 19, 3:41 AM

Wed, Jul 18

avt77 updated the diff for D47196: [Time-report ](2): Recursive timers in Clang.

I added required comments and did the required changes.

Wed, Jul 18, 7:09 AM
avt77 created D49477: [X86] Improved sched models for X86 BSWAP* instructions..
Wed, Jul 18, 5:34 AM

Tue, Jul 17

avt77 updated the diff for D48222: Check Sched Class tables at generation time - 2.

I replaced 'auto' with real types accordingly to Simon's request.

Tue, Jul 17, 10:04 AM
avt77 added a comment to D48222: Check Sched Class tables at generation time - 2.

What happened to the BSWAP patch? Compared to BT (D49243) it should have been pretty trivial to implement.

In fact we have there very similar issues but even with rr versions: we should separate implementations for different sizes 16/32/64. I'm going to complete it tomorrow. But again - most probably it will be w/o memory operands support like in D49243.

Tue, Jul 17, 8:26 AM
avt77 updated the diff for D49243: [X86] Improved sched models for X86 BT*rr instructions.

Now we use WriteRes instead of *WriteResPair.
Folded versions of the intrs will be implemented in the next patches.

Tue, Jul 17, 6:21 AM
avt77 added a comment to D49243: [X86] Improved sched models for X86 BT*rr instructions.

http://www.agner.org/optimize/instruction_tables.pdf, page 202, "Intel Haswell", "List of instruction timings and μop breakdown" appears to list all the BT* as having latency of 1.

I mixed columns for latency and throughput: latency is missed.

Tue, Jul 17, 2:46 AM
avt77 added a comment to D48222: Check Sched Class tables at generation time - 2.

(Would be great if the checkbox 'done' in notes would be getting checked, too)

Tue, Jul 17, 2:28 AM
avt77 added a comment to D47196: [Time-report ](2): Recursive timers in Clang.

Adding startFrontendTimer/stopFrontendTimer helps a little, but it's still difficult to match a given startFrontendTimer to the corresponding stopFrontendTimer because they're in completely different functions in some cases. Do they really need to be scattered like that? If they do, please add comments so someone reading the code can match them up.

Tue, Jul 17, 1:16 AM

Mon, Jul 16

avt77 added a comment to D49243: [X86] Improved sched models for X86 BT*rr instructions.

Hi All,
I need your help!

Mon, Jul 16, 7:40 AM
avt77 updated the diff for D48222: Check Sched Class tables at generation time - 2.

It seems I fixed all issues raised by lebedev.ri. The open question is: should I remove the second check inside checkSchedClasses? I'm sure there is a way to get default values for such SchedWrites like WriteALU, WriteFAdd, etc. but I don't know at the moment how to do it. Please, help.

Mon, Jul 16, 7:31 AM
avt77 added inline comments to D48222: Check Sched Class tables at generation time - 2.
Mon, Jul 16, 7:23 AM
avt77 added inline comments to D48222: Check Sched Class tables at generation time - 2.
Mon, Jul 16, 1:53 AM
avt77 added inline comments to D49243: [X86] Improved sched models for X86 BT*rr instructions.
Mon, Jul 16, 12:41 AM

Fri, Jul 13

avt77 updated the diff for D49243: [X86] Improved sched models for X86 BT*rr instructions.

I renamed WriteBTr with WriteBitTest.
I could not add memory version of the instructions because there were some issues. For example, if we have

Fri, Jul 13, 7:45 AM
avt77 added a comment to D49243: [X86] Improved sched models for X86 BT*rr instructions.

I'm assuming that you have run ninja check-llvm-tools-llvm-mca-x86 and it was ok. (the test coverage seems to be ok as-is.)

Fri, Jul 13, 2:06 AM

Thu, Jul 12

avt77 created D49243: [X86] Improved sched models for X86 BT*rr instructions.
Thu, Jul 12, 8:30 AM
avt77 updated the diff for D48222: Check Sched Class tables at generation time - 2.

Now the patch keeps only infrastructure changes to produce new TableGen warns about sched models.

Thu, Jul 12, 3:08 AM

Wed, Jul 11

avt77 added a comment to D48222: Check Sched Class tables at generation time - 2.

Please can you pul out the BSWAP change into its own patch for review?

Wed, Jul 11, 7:50 AM

Tue, Jul 10

avt77 updated the diff for D47196: [Time-report ](2): Recursive timers in Clang.

I fixed all issues raised by efriedma: GlobalDecl(FD), function body, class names, etc. Many tnx for your help.

Tue, Jul 10, 7:26 AM
avt77 added a comment to D47196: [Time-report ](2): Recursive timers in Clang.

Can you give an example of what the output looks like?

Tue, Jul 10, 7:21 AM

Mon, Jul 9

avt77 updated the diff for D48222: Check Sched Class tables at generation time - 2.

I fixed warns for BSWAP instrs (see changes in X86*.td files). If this approach is OK I'll fix other instrs.

Mon, Jul 9, 5:28 AM
avt77 added a comment to D48222: Check Sched Class tables at generation time - 2.

At the moment we have the following warns for X86:

Mon, Jul 9, 3:56 AM
avt77 added a comment to D48222: Check Sched Class tables at generation time - 2.

In fact we have the warnings for 2 Targets only:

Mon, Jul 9, 1:33 AM
avt77 added a comment to D48222: Check Sched Class tables at generation time - 2.

@avt77 Next step is probably to start briefly collating the instructions causing the warnings and investigate how best to fix them (list them here, raise bugs etc.). The aim would be to see if we can remove ALL these warnings before we consider committing this patch - its probably time to add the maintainers for each target that has warnings so they can be investigated?

Mon, Jul 9, 12:14 AM

Tue, Jul 3

avt77 updated the diff for D48222: Check Sched Class tables at generation time - 2.

Now it's almost completed but should deal with deafult Latency value: hope to implement it soon. (The current version does not produce yet warning about identical uOps & Latency during compiler build: maybe it's OK?)

Tue, Jul 3, 7:25 AM
avt77 added a comment to D48222: Check Sched Class tables at generation time - 2.

Hi! I was out for 2 weeks that's why I did not do anything here. Is it still interesting for you? I'm going to publish an update asap.
But the question is: how to get latnency/uop from CodeGenSchedClass ?

Tue, Jul 3, 4:12 AM

Jun 15 2018

avt77 created D48222: Check Sched Class tables at generation time - 2.
Jun 15 2018, 9:07 AM

Jun 7 2018

avt77 added a comment to D47766: Removing unsupported resources from sched model.

9>>! In D47766#1124708, @RKSimon wrote:

I really don't like the idea of a separate file - I think the models need to stay self contained. @courbet 's approach in D47763 seems a lot tidier

Jun 7 2018, 6:55 AM
avt77 added a comment to D47763: [X86] Explicitly mark unsupported zmm classes in scheduling models..

Introduce X86WriteRes(|Pair)Unsupported.

Jun 7 2018, 6:36 AM
avt77 added a comment to D47637: Check Sched Class tables at generation time.

If I use one check only (" // Check if an instruction is always overriden (candidate for a new class?)") I see warnings for p9model only.
At the moment, I'm learning debug output related to generation of all these tables and hope to come up with some realistic logging soon.

Jun 7 2018, 6:13 AM
avt77 added a comment to D47637: Check Sched Class tables at generation time.

Simon, I have some troubles with slack connection that's why I read your message only now :-( I'll back with answer asap.

Jun 7 2018, 6:05 AM
avt77 added a comment to D47766: Removing unsupported resources from sched model.

Then I'm not sure if we win a lot from this as it makes things less explicit, though I agree that it slightly reduces code duplication.

Jun 7 2018, 1:01 AM

Jun 6 2018

avt77 added a comment to D47766: Removing unsupported resources from sched model.

Of course not. First of all, I suggest to put AVX2 and AVX512 instructions in separate files and to use them in models which don't support AVX2 and/or AVX512. Etc.

Jun 6 2018, 8:50 AM
avt77 updated the diff for D47766: Removing unsupported resources from sched model.

Obviously, the idea was to use one include file for all targets. But it did not work in the first version of the patch. Now I did a "dirty hack" in TableGen to be able to do it. It works for 2 CPUs and obviously it should work for others. The usage is very simply: the common include file should keep the common unsupported instructions while the specific onces should be inserted into the specific .td files.

Jun 6 2018, 8:13 AM

Jun 5 2018

avt77 added a comment to D47637: Check Sched Class tables at generation time.

And the last question. Again, in P9Model we have

Jun 5 2018, 8:02 AM
avt77 added a comment to D47637: Check Sched Class tables at generation time.

Next, we have for P9Model only:

Jun 5 2018, 7:58 AM
avt77 added a comment to D47637: Check Sched Class tables at generation time.

Something is wrong with current diagnostic. Fro example, we have

Jun 5 2018, 7:39 AM
avt77 created D47766: Removing unsupported resources from sched model.
Jun 5 2018, 3:56 AM

Jun 4 2018

avt77 updated the diff for D47196: [Time-report ](2): Recursive timers in Clang.

To simplify the review I removed all LLVM_DEBUG items - now the patch is really shorter than it was before.

Jun 4 2018, 5:35 AM

Jun 1 2018

avt77 created D47637: Check Sched Class tables at generation time.
Jun 1 2018, 8:17 AM

May 30 2018

avt77 added a comment to D47196: [Time-report ](2): Recursive timers in Clang.

Hi All,
What should I do to simplify the review?
I could remove all LLVM_DEBUG related stuff; I could remove all addtional counters and leave only necessary one or two of them. As result the patch will become shorter.

May 30 2018, 2:08 AM

May 24 2018

avt77 updated the diff for D47196: [Time-report ](2): Recursive timers in Clang.

The sources were re-based to fit in LLVM_DEBUG rename.
One more counter was added.
Debug logging was improved again.

May 24 2018, 5:53 AM

May 23 2018

avt77 updated the diff for D47196: [Time-report ](2): Recursive timers in Clang.
  • switched to llvm::sort - from std::sort (tnx to mgrang)
  • added a new counter (hope it will improve the output numbers)
  • slightly changed a couple of existing counters
  • seriously improved debug/monitor logging
May 23 2018, 8:35 AM

May 22 2018

avt77 created D47196: [Time-report ](2): Recursive timers in Clang.
May 22 2018, 7:06 AM

Apr 24 2018

avt77 added a comment to D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature.

buildbots have been failing all day (and were still failing). So I pushed my suggested fix.
I hope that was OK.

Apr 24 2018, 12:04 AM

Apr 23 2018

avt77 added a comment to D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature.

I can't see that it has been reverted.
But I guess that the table maybe is sorted based on time spent in each pass? So that is why it might be sorted differently on different buildbots (or when using pipe etc).

So I think a quick fix is to add -DAG to the checks that can be reorder and submit that fix.

Apr 23 2018, 8:53 AM
avt77 added a comment to D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature.

Anyway, if the order isn't deteministic, then a solution could be to use CHECK-DAG instead of CHECK for the checks that may be reordered. For example:

Thank you very much! I'll try to fix it in this way.

Apr 23 2018, 8:06 AM
avt77 added a comment to D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature.

In any case, when you see a test failing on bots and the fix isn't obvious,
revert first to get the bots back green.

Apr 23 2018, 8:03 AM
avt77 reopened D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature.

It's terrible but my new test was failed again as result of commit of this patch!

Apr 23 2018, 5:54 AM
avt77 committed rC330571: Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm….
Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm…
Apr 23 2018, 2:25 AM
avt77 committed rL330571: Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm….
Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm…
Apr 23 2018, 2:25 AM
avt77 closed D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature.
Apr 23 2018, 2:25 AM

Apr 20 2018

avt77 updated the diff for D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature.

I fixed issues raised by efriedma.

Apr 20 2018, 7:19 AM
avt77 added inline comments to D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature.
Apr 20 2018, 7:18 AM

Apr 19 2018

avt77 added a comment to D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature.

Hi All,
Are there other issues related to this patch?
If NO could anyone give me LGTM? I need this patch committed to continue with recursive time counters.

Apr 19 2018, 1:29 AM

Apr 17 2018

avt77 updated the diff for D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature.

I moved FrontendTiming.cpp from lib/Basic/ to lib/Frontend/.

Apr 17 2018, 4:19 AM

Apr 16 2018

avt77 added a comment to D43578: -ftime-report switch support in Clang.

I think we need to fix the overlap issue as a prerequisite to adding timers with large amounts of overlap, especially self-overlap. Otherwise the numbers will likely do more harm than good, as they will significantly misattribute runtime. Fortunately, I think that should only require some relatively small changes to the LLVM timer infrastructure.

JFYI: the given patch introduces non-recursive (self-overlaps) timers only. I'm going to introduce self-overlaps timers when D45619 is committed.

Apr 16 2018, 1:51 AM
avt77 added a comment to D43578: -ftime-report switch support in Clang.

I think D45619 is a good change, and I'd like to see that get committed.

Could you give me LGTM in this case? I'm going to publish "recursive"(ovelaped) timers but I'd like to base it on D45619.

Apr 16 2018, 1:43 AM

Apr 14 2018

avt77 added a comment to D43578: -ftime-report switch support in Clang.

Last time I looked at doing this, I found that LLVM's timer infrastructure was fundamentally unsuitable for adding timers like these to Clang.

Thank you for this answer. As I understand we should close both this review and D45619 but you'd like to see something like MS has here, right?

Apr 14 2018, 12:20 AM
avt77 added inline comments to D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature.
Apr 14 2018, 12:02 AM

Apr 13 2018

avt77 created D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature.
Apr 13 2018, 5:56 AM

Apr 12 2018

avt77 added a comment to D43578: -ftime-report switch support in Clang.

It wasn't my suggestion. @thakis wrote: "We probably should have a separate bool in clang and key this off that and make -ftime-report set both."

Apr 12 2018, 11:09 AM
avt77 updated the diff for D43578: -ftime-report switch support in Clang.

I removed the dependence on TimePassesIsEnabled (as @davezarzycki sugested) and fixed the issue with failed test (tnx to @russell.gallop). As result the patch was redesigned.

Apr 12 2018, 9:17 AM

Apr 11 2018

avt77 added a comment to D43578: -ftime-report switch support in Clang.

We also see an assertion failure prior to the revert. At r329738:

Apr 11 2018, 7:23 AM
avt77 added a comment to D43578: -ftime-report switch support in Clang.

@davezarzycki remarks in https://reviews.llvm.org/D45485 that this breaks the shared build. The proposed fix there is to make several of clang's modules depend on LLVM's IR library ("Core"). This seems weird to me for two reasons, one architectural, one form a build point of view:

  1. The modules growing the dep don't really depend on IR, they just need that one bool that happens to be defined there. That bool is called TimePassesIsEnabled which is a reasonable bool to live in IR, but this patch starts using that bool for meanings other than "should we time passes?". It instead uses the same bool to decide if clang should print a bunch of timing info. We probably should have a separate bool in clang and key this off that and make -ftime-report set both.
  2. From a build PoV, depending on Core means explicitly depending on TableGen processing the Attributes.td and Intrinsics.td files in include/llvm/IR, which needlessly (explicitly) serializes the build.
Apr 11 2018, 3:18 AM

Apr 10 2018

avt77 added a comment to D43578: -ftime-report switch support in Clang.

Reverted:

Apr 10 2018, 12:06 PM
avt77 added a comment to D43578: -ftime-report switch support in Clang.

A revert in practice just means undoing the changes. For example, while both git and svn have revert subcommands, you can also just take the original diff/patch, pipe it into patch -R, and if there are no conflicts, commit the result.

Apr 10 2018, 11:28 AM
avt77 reopened D43578: -ftime-report switch support in Clang.

Obviously, this patch was not ready for commit that's why I reopen the revison.
But I don't know how to revert the patch from trunk: please, help me.

Apr 10 2018, 10:50 AM
avt77 added a comment to D43578: -ftime-report switch support in Clang.

Could you help me with revert of the committed patch?
I'll do all necessary changes but I don't know how I can revert the patch.

Apr 10 2018, 10:45 AM
avt77 committed rC329714: I removed the failed test..
I removed the failed test.
Apr 10 2018, 8:48 AM
avt77 committed rL329714: I removed the failed test..
I removed the failed test.
Apr 10 2018, 8:48 AM
avt77 committed rC329693: The test was fixed..
The test was fixed.
Apr 10 2018, 5:20 AM
avt77 committed rL329693: The test was fixed..
The test was fixed.
Apr 10 2018, 5:20 AM
avt77 committed rL329684: -ftime-report switch support in Clang..
-ftime-report switch support in Clang.
Apr 10 2018, 3:37 AM
avt77 committed rC329684: -ftime-report switch support in Clang..
-ftime-report switch support in Clang.
Apr 10 2018, 3:37 AM
avt77 closed D43578: -ftime-report switch support in Clang.
Apr 10 2018, 3:37 AM

Apr 9 2018

avt77 updated the diff for D43578: -ftime-report switch support in Clang.

I fixed all issues raised by mzolotukhin and answered on his questions. I'm waiting for LGTM now.

Apr 9 2018, 5:15 AM
avt77 added inline comments to D43578: -ftime-report switch support in Clang.
Apr 9 2018, 2:07 AM
avt77 abandoned D44559: [Sema] Wrong width of result of mul operation.

It is not a bug.

Apr 9 2018, 12:27 AM · Restricted Project
avt77 abandoned D24760: Failure to hoist constant out of loop.

The trunk fixed the issue.

Apr 9 2018, 12:27 AM

Mar 30 2018

avt77 added a comment to D44559: [Sema] Wrong width of result of mul operation.

In fact I think about mul only because of X86 nature: it always put the mul result in wider place (than its args). (Don't know about other CPUs - maybe the same?) As result we could change the current design to reflect this feature:

Mar 30 2018, 1:31 AM · Restricted Project

Mar 29 2018

avt77 added a comment to D44559: [Sema] Wrong width of result of mul operation.

You are welcome to start a thread on cfe-dev to gather support for changing the design of -Wconversion to always warn about the potential for overflow in sub-int multiplication.

Mar 29 2018, 12:38 AM · Restricted Project

Mar 27 2018

avt77 added a comment to D44559: [Sema] Wrong width of result of mul operation.

If that operation overflows, so be it — we're not going to warn about the potential for overflow every time the user adds two ints, and by the same token, it doesn't make any sense to warn about every time the user adds two shorts just because the language made this otherwise-unimportant technical decision to do the arithmetic in a wider type.

Mar 27 2018, 5:10 AM · Restricted Project

Mar 26 2018

avt77 added a comment to D44559: [Sema] Wrong width of result of mul operation.

No, I still oppose this patch.

Mar 26 2018, 4:11 AM · Restricted Project

Mar 22 2018

avt77 updated the diff for D40602: [X86] Add MC level selection support for SHLD (64-bit only).

I re-based sources to reflect commit of D43813.

Mar 22 2018, 2:38 AM
avt77 added a comment to D44559: [Sema] Wrong width of result of mul operation.

That's an interesting question. In general, these warnings do try to ignore the effects of implicit promotion. We would not want -Wsign-conversion to fire on unsigned short x = an_unsigned_short + 1; (or - 1, for that matter), even though formally this coerces a signed int to unsigned short. Similarly, -Wsign-conversion *should* warn on signed short x = an_unsigned_short + 1;, even though formally the promotion from unsigned short to signed int is not problematic and the final conversion from signed int to signed short is not a signedness change. (This second example should also generate a -Wconversion warning, but the questions are independent.) Applying that strictly here would say that the user is entitled to think of this as an operation on unsigned char that then gets losslessly promoted to signed short, even though arithmetically that's not what happens. On the other hand, I do think there's some room for -Wsign-conversion to be more aggressive than -Wconversion about this sort of thing; -Wsign-conversion should generally fire for any changes in signedness from the original operand types (with the usual complexities around constant values), and there's just an exception for computations whose value is known to fit within the expressible range of the result type, which is not true of this multiplication. So I think it would be acceptable to warn on this.

John.

Mar 22 2018, 12:02 AM · Restricted Project

Mar 21 2018

avt77 updated the diff for D44559: [Sema] Wrong width of result of mul operation.

I removed the dependence on arch and updated the tests.

Mar 21 2018, 3:39 AM · Restricted Project
avt77 added a comment to D44559: [Sema] Wrong width of result of mul operation.

I think we're correct not to warn here and that GCC/ICC are being noisy. The existence of a temporary promotion to a wider type doesn't justify warning on arithmetic between two operands that are the same size as the ultimate result. It is totally fair for users to think of this operation as being "closed" on the original type.

Could you please clarify, are you saying that PR35409 is not a bug, and clang should continue to not warn in those cases?

Correct.

Mar 21 2018, 1:26 AM · Restricted Project

Mar 16 2018

avt77 committed rL327721: This patch fixes the invalid usage of OptSize in Machine Combiner..
This patch fixes the invalid usage of OptSize in Machine Combiner.
Mar 16 2018, 9:09 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Mar 16 2018, 9:08 AM
avt77 added a comment to D43813: [Machine Combiner] Valid use of OptSize.

LGTM with a couple of style comments - pity we can't get a test case

Mar 16 2018, 9:03 AM
avt77 created D44559: [Sema] Wrong width of result of mul operation.
Mar 16 2018, 5:53 AM · Restricted Project
avt77 closed D42728: Add more warnings for implict conversions (e.g. double truncation to float)..

It's committed: https://reviews.llvm.org/rC327618.

Mar 16 2018, 5:43 AM
avt77 added a comment to D42728: Add more warnings for implict conversions (e.g. double truncation to float)..

I fixed all issues raised by ABataev and committed the patch as revision 327618. I'll close the review in some hours.

You are aware that you can either use arc patch (best way), or at least manually add Differential revision: https://reviews.llvm.org/Dsomething line to the commit msg, so

  1. the differential is auto-closed
  2. the commit msg is taken from differential description
  3. there is a trivial way to go from commit to differential (and vice versa) ?
Mar 16 2018, 3:53 AM