copypasta doc of InlinerStats class
/ \brief Calculate and dump ThinLTO specific inliner stats.
/ The main statistics are:
/ (1) Number of inlined imported functions,
/ (2) Number of imported functions inlined into importing module (indirect),
/ (3) Number of non imported functions inlined into importing module (indirect).
/ The difference between first and the second is that first stat counts
/ all performed inlines on imported functions, but the second one only the
/ functions that have been eventually inlined to a function in the importing
/ module (by a chain of inlines). Because llvm uses bottom-up inliner, it is
/ possible to e.g. import function A, B and then inline B to A,
/ and after this A might be too big to be inlined into some other function
/ that calls it. It calculates this statistic by building graph, where
/ the nodes are functions, and edges are performed inlines and then by marking
/ the edges starting from not imported function.
/
/ If Verbose is set to true, then it also dumps statistics
/ per each inlined function, sorted by the greatest inlines count like
/ - number of performed inlines
/// - number of performed inlines to importing module
Details
Diff Detail
Event Timeline
include/llvm/Transforms/IPO/InlinerStats.h | ||
---|---|---|
12 ↗ | (On Diff #64429) | Perhaps instead of referring to these as a "real inline" it would be clearer to say "inline to importing module" or something more descriptive like that. |
include/llvm/Transforms/IPO/InlinerStats.h | ||
---|---|---|
22 ↗ | (On Diff #64429) | indeed. I am gonna change that comment to be more clear |
55 ↗ | (On Diff #64429) | because I am moving all the pairs from map to vector, so either I will do a copy of everything or I will move all the stuff |
59 ↗ | (On Diff #64429) | Yes, but this label splits private functions from private members. It is not necessary, but the code is more readable this way. |
lib/Transforms/IPO/Inliner.cpp | ||
96 | exactly, the fields are set to null after inlining | |
lib/Transforms/IPO/InlinerStats.cpp | ||
17 ↗ | (On Diff #64429) | Ok so there are two things
|
lib/Transforms/IPO/InlinerStats.cpp | ||
---|---|---|
17 ↗ | (On Diff #64429) | How about use a set instead of a list then. It avoids the need to sort. I'm not sure on why you need a count of how many times it was pushed (inlined into)? Since you keep appending the functions inlined into each function here in its InlinedFunctions list, why do you need to invoke dfs() on the caller each time? |
lib/Transforms/IPO/InlinerStats.cpp | ||
---|---|---|
17 ↗ | (On Diff #64429) | yes, that is true, I was wrong. I will just sort it and unique it. It will be much faster than using set. |
include/llvm/Transforms/IPO/InlinerStats.h | ||
---|---|---|
1 ↗ | (On Diff #64426) | Transforms/Utils is a better place to have these two files. I will also rename it to ThinLTOInliner stats because it is mostly specific to ThinLTO. Actually, you can get rid of the header file and declare getInlinerStatistics in, say, InlinerPass.h and move the class to the .cpp file. |
10 ↗ | (On Diff #64426) | "It calculates and dumps the following stats" is more readable |
16 ↗ | (On Diff #64429) | "inlined to not imported function" -> "eventually inlined to a function in the importing module" |
19 ↗ | (On Diff #64429) | "the real values" -> "this statistic" |
lib/Transforms/IPO/InlinerStats.cpp | ||
12 ↗ | (On Diff #64429) | Naming the arguments Caller and Callee makes it consistent with the terminology used in the inliner |
72 ↗ | (On Diff #64429) | I think the graph can have cycle. Consider a mutual recursion A->B->A. Let's say B gets inlined into A. This would normally result in a self-recursion (A->A). This would prevent A from being inlined into B. But, if we are in a situation where B first gets inlined into A and the A->A call is DCEd. Then A could be inlined into B causing a cycle in your InlineGraph. |
73 ↗ | (On Diff #64429) | Let's say A and B are local functions (functions in the importing module) and C is an imported function and there is a call chain A->B->C. Let's say C first gets inlined into B and then B gets inlined into A. You'll have nodes for A, B and C and A and B are in the NonExternalFunctions. You'll dfs from both A and B. NumberofRealInlines of B will be 2 which not what you want. |
include/llvm/Transforms/IPO/InlinerStats.h | ||
---|---|---|
2 ↗ | (On Diff #64429) | actually not right now, because I call member functions. If this is good and nobody will disagree then I can change it like that. |
lib/Transforms/IPO/InlinerStats.cpp | ||
73 ↗ | (On Diff #64429) | yes, I am fixing it right now. |
include/llvm/Transforms/IPO/InlinerStats.h | ||
---|---|---|
22 ↗ | (On Diff #64429) | OK for a side structure, does it need to build a graph? I'm not sure if a map F -> { real, unreal } + the global counters would not suffice? I mean, it depends on the semantics of these counters, and I'm still not sure about this. For example: A->B->C and D->B->C, with C and B the only imported functions. If we import entirely (C into B, and B into A and D), what is expected to be computed? |
include/llvm/Transforms/IPO/InlinerStats.h | ||
---|---|---|
22 ↗ | (On Diff #64429) | Then the number of inlines for B: 2, for C: 1, and number of inline to importing module (aka real inlines) will be B: 2, C: 2. The graph is to calculate the number of real inlines. Without it we won't know if the function was realy inlined into some function that wasn't imported and we won't be able to see for example |
include/llvm/Transforms/IPO/InlinerStats.h | ||
---|---|---|
22 ↗ | (On Diff #64429) | Something does not line up, the description in the revision is "The difference between first and the second is that first stat counts all performed inlines, and second one only the functions that really have been inlined to some not imported function." This sentence makes me think that the first number is always greater than the second, however you provided 1 and 2 for C. |
include/llvm/Transforms/IPO/InlinerStats.h | ||
---|---|---|
22 ↗ | (On Diff #64429) | Yes, I also though so when designing it, but because the (1) only counts direct inlines (how many times this function have been inlined), and (2) counts indirect inlines to not imported function, then in example like you have shown it might happen that (2) > (1). I am fixing the code also to handle the cases like: Adding X->A and X->D to your example, where X is also not imported function and A and D have been inlined to X should give us Right now it would give because I don't start only from "roots" (not imported functions that have not been inlined anywhere). |
include/llvm/Transforms/IPO/InlinerStats.h | ||
---|---|---|
22 ↗ | (On Diff #64429) | Thanks for clarifying. So, according to you example, it seems that we'll count the number of "real" inline for an imported function only once (I.e. at the boundary between non-imported and imported). Because in the final optimized binary, we would codegen X and A and D that both contains C (and X contains two copies of C), so 4, but the counter for C is only 2. Also what is X is an imported function? |
lib/Transforms/IPO/InlinerStats.cpp | ||
---|---|---|
51 ↗ | (On Diff #64429) | s/not external/not imported/ Also, it would be useful to know here whether the caller that we are inlining into is imported or not. |
61 ↗ | (On Diff #64429) | Should be trivial I think to compute the reverse as well: how many imported functions were not inlined into a non-imported function (possibly via other inlines). |
include/llvm/Transforms/IPO/InlinerStats.h | ||
---|---|---|
22 ↗ | (On Diff #64429) | I think it is more important to simply flag whether >0 of the inlines are "real", perhaps we don't really want to try to count # of real inlines, or it gets tricky to define compared to the total number of inlines as shown above. |
lib/Transforms/IPO/InlinerStats.cpp | ||
---|---|---|
17 ↗ | (On Diff #64429) | I changed the name to NonImportedCallers |
lib/Transforms/IPO/InlinerStats.cpp | ||
---|---|---|
51 ↗ | (On Diff #64429) | But we can have multiple callers. I think the real inlines gives you the information that you need. |
61 ↗ | (On Diff #64429) | I am not sure if I should compute it here. The number of all imported functions is counted in importer. |
72 ↗ | (On Diff #64429) | fixed. |
include/llvm/Transforms/IPO/InlinerStats.h | ||
---|---|---|
43 ↗ | (On Diff #64801) | Maybe "Number of inlines into non imported function (possibly indirect via intermediate inlines)" |
lib/Transforms/IPO/Inliner.cpp | ||
96 | Maybe then change comment to say "Callee and Caller will be set to null in CS after inlining" | |
lib/Transforms/IPO/InlinerStats.cpp | ||
22 ↗ | (On Diff #64801) | s/form/from/ |
24 ↗ | (On Diff #64801) | Even with imported functions, by not putting intra-module inlines in the graph it should reduce the size and the traversal overhead. Although I would expect that most intra module inlines would have been done already in the compile step. Maybe just note that in your comment. |
52 ↗ | (On Diff #64801) | Oh got it, I missed the fact that we only have the callee info here and aren't walking from the caller. |
57 ↗ | (On Diff #64801) | I would remove this early exit as it makes assumptions about the sorting done, and if the sorting algorithm changes this would need to change too. Maybe just continue so we don't get a message under EnableListStats below when there are no inlines. |
70 ↗ | (On Diff #64801) | s/inlined imported functions to/imported functions inlined into/ |
72 ↗ | (On Diff #64801) | s/inlined not imported functions to/non-imported functions inlined into/ |
lib/Transforms/IPO/InlinerStats.cpp | ||
---|---|---|
24 ↗ | (On Diff #64801) | yea, but if you just pass those flags to see the statistics for non thinlto then it is useless to compute it by graph. I gonnna mention it. |
include/llvm/Transforms/IPO/InlinerStats.h | ||
---|---|---|
23 ↗ | (On Diff #64801) | Ok right now it works exactly like Teresa says. Is it all clear Mehdi? |
include/llvm/Transforms/Utils/InlinerStats.h | ||
---|---|---|
1 ↗ | (On Diff #64978) | As discussed offline, incorporate ThinLTO to the name or at the minimum add comments to indicate that the primary intent is to use with ThinLTO. Perhaps rename it to InlinerFunctionImportStats.h - as used in the option name. |
16 ↗ | (On Diff #64978) | Nit: I prefer recordInlining or similar small name, but will leave it to you |
lib/Transforms/IPO/Inliner.cpp | ||
60 | use the string "Basic" instead of an empty string | |
96 | You can not call getCallee() or getCalledFunction() because the call instruction does not exist after inlining and those methods access methods of the call instruction. As an aside, this comment is not specific to this patch and typically could just add the comment separately (which doesn't need any review). I don't mind it being part of this patch though. | |
lib/Transforms/Utils/InlinerStats.cpp | ||
25 ↗ | (On Diff #64978) | typo: imporrted -> imported |
31 ↗ | (On Diff #64978) | merge the next line here |
38 ↗ | (On Diff #64978) | Comment out of sync with the code |
55 ↗ | (On Diff #64978) | I don't expect the number to exceed max value of int16_t in real programs, but perhaps possible in generated test cases. Why not make it int32_t ? |
193 ↗ | (On Diff #64978) | As discussed offline, this is not threadsafe |
include/llvm/Transforms/Utils/ThinLTOInlinerStats.h | ||
---|---|---|
1 ↗ | (On Diff #64999) | "Generating inliner statistics for imported functions" |
16 ↗ | (On Diff #64999) | doxygen comment |
17 ↗ | (On Diff #64999) | s/InlinerStatistics/ThinLTOInlinerStatistics/ (suggested below) |
lib/Transforms/IPO/InlinerStats.cpp | ||
61 ↗ | (On Diff #64429) | Yes but then the client has to find both stats and subtract rather than get the stat immediately. I think it would be useful to walk over the functions here, and simply count how many have the imported metadata tag, then emit that info here, along with the percentage of imported funcs that were inlined into the importing module and that weren't. |
lib/Transforms/Utils/ThinLTOInlinerStats.cpp | ||
1 ↗ | (On Diff #64999) | "Generating inliner statistics for imported functions" is clearer. If too long, probably clarify further down in the file head comment block. |
26 ↗ | (On Diff #64999) | Here and in below line, I think it is clearer to restructure like "Number of imported functions inlined into...". |
42 ↗ | (On Diff #64999) | s/InlinerStatistics/ThinLTOInlinerStatistics/ |
91 ↗ | (On Diff #64999) | What is the multithreaded case we are concerned about? Is it when the ThinLTO backend threads are launched in parallel via a ThreadPool? In that case we want each thread to have a completely independent set of stats, which I don't think this mutex provides. |
lib/Transforms/Utils/ThinLTOInlinerStats.cpp | ||
---|---|---|
26 ↗ | (On Diff #64999) | wtf I was pretty sure I changed that. |
91 ↗ | (On Diff #64999) | I am not sure if there is any problem right now, but we also probably don't want to risk breaking it in the future. The overhead should not be very big and it won't be runned frequently. |
I prefer the middle end to stay as much agnostic of the exact "use case" (ThinLTO) and focus on the "feature" from a library point of view: so I'd rather see "functionimport" used instead of "ThinLTO".
I.e. any system that would use some importing mechanism to implement a runtime in a JIT environment may be interested in this. We design LLVM as reusable and independent components.
lib/Transforms/Utils/ThinLTOInlinerStats.cpp | ||
---|---|---|
92 ↗ | (On Diff #65117) | We don't add mutexes without motivation. You need to explain why *this* one makes sense (I.e. how it could get use in a multi-threaded environment in the future). |
lib/Transforms/Utils/ThinLTOInlinerStats.cpp | ||
---|---|---|
92 ↗ | (On Diff #65117) | ok so we discussed it offline, and the main problem here is that multiple ThinLTO threads would use it. The solution is to create new InlinerStatistic object in Inliner and remove mutexes. |
lib/Transforms/Utils/ThinLTOInlinerStats.cpp | ||
---|---|---|
203 ↗ | (On Diff #65117) | Global mutable state is seldom the right choice, I assume this is what you're gonna move to be state in the inliner? |
lib/Transforms/Utils/ThinLTOInlinerStats.cpp | ||
---|---|---|
203 ↗ | (On Diff #65117) | Exactly |
Looks pretty close. Just some minor comments below.
lib/Transforms/IPO/Inliner.cpp | ||
---|---|---|
597 | This indentation doesn't look right to me - make sure you clang format. | |
lib/Transforms/Utils/ImportedFunctionsInliningStatistics.cpp | ||
10 | s/usefull/useful/ | |
51 | Add empty line above. | |
132 | suggest adding "anywhere" at the end to distinguish from below stat. | |
137 | Make this consistent with other lines, e.g. maybe "non-imported functions inlined anywhere". | |
test/Transforms/Inline/inline_stats.ll | ||
18 | It isn't clear what the percentages represent (e.g. some are percent of total # functions and some are percent of inlined functions). Probably need to add a tag - e.g.: ; CHECK: inlined functions: 5 [50% of all functions] However, after typing that out, I think for the stat relating to how many are inlined into the importing module, it would be much more useful to have the percentage in terms of the total number of that type of function. E.g. instead of expressing the "imported functions inlined into importing module" as a percentage of "imported functions inlined", it would be more useful to have it be a percentage of "imported functions". Ditto for non-imported functions inlined into importing module. |
include/llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h | ||
---|---|---|
10 | s/usefull/useful/ |
test/Transforms/Inline/inline_stats.ll | ||
---|---|---|
18 | Could you add one more stat - the # of imported functions not inlined into importing module (along with percentage of all imported functions)? I know it is computable from the other stats, but it would be useful to be able to get this directly. |
LGTM, other than one missing stat I suggested adding below. Please wait for Easwaran's remaining comments though.
test/Transforms/Inline/inline_stats.ll | ||
---|---|---|
18 |
Please add this one. |
lib/Transforms/IPO/Inliner.cpp | ||
---|---|---|
597 | This was actually formatted with clang-format. I guess the arguments are not in the same column because it would look the same as the next arguments to function. | |
test/Transforms/Inline/inline_stats.ll | ||
18 | I am not sure if it is good way, but I will change it. What I am woried is that the last stat won't tell much at the fist glance, because the number will be low | |
19 | you mean substract imported functions from inlined imported functions? like |
test/Transforms/Inline/inline_stats.ll | ||
---|---|---|
19 | Meaning subtract "imported functions inlined into importing module" from "imported functions" (4 in the above example). | |
19 | I'm less sure about how we will use the last one (non-imported functions inlined into importing module vs non-imported functions inlined anywhere), but for the imported functions what we primarily want to see is how many/what percentage were inlined into the importing module. |
Generally looks good. Most of the comments are minor. Please add a test case for the non-verbose case.
lib/Transforms/IPO/Inliner.cpp | ||
---|---|---|
65 | "Inliner stats for imported functions" or something like that. Since the option name does not have ThinLTO, remove it from the description. | |
96 | To reiterate, the problem is that the call instruction does not exist and calling any methods on that CallInst accesses freed memory. CallSite does not explicitly store Caller and Callee and so there is no question of "Callee and Caller will be set to null". | |
lib/Transforms/Utils/CMakeLists.txt | ||
19 | Indentation. | |
lib/Transforms/Utils/ImportedFunctionsInliningStatistics.cpp | ||
25 | Inliner contains an object of ImportedFunctionsInliningStatistics. By default you are not generating statistics and yet this will call reserve unnecessarily. Not a big deal - we are talking about <2KB per module compilation here, but still unnecessary. | |
107 | Node.second.NumberOfInlines is > 0 here, so you don't need the (Node.second.NumberOfInlines > 0) * 1. You could simply increment the count by 1. | |
109 | No need to do * 1 | |
test/Transforms/Inline/inline_stats.ll | ||
2 | This doesn't test the non-verbose case. Just use this test case and use a different filecheck prefix. |
lib/Transforms/Utils/ImportedFunctionsInliningStatistics.cpp | ||
---|---|---|
107 | I know about this, I wanted to make it more explicit. The implicit conversion from bool is bugprone, so I will just add int() here so | |
test/Transforms/Inline/inline_stats.ll | ||
19 | Is this line ok? imported functions inlined into importing module: 3 [42.86% of imported functions], left: 4 [57.14% of imported functions] |
test/Transforms/Inline/inline_stats.ll | ||
---|---|---|
20 | Sure, or "remaining", or "not inlined" to be explicit |
lib/Transforms/IPO/Inliner.cpp | ||
---|---|---|
100 | New comment doesn't make sense. I think you missed a word or two? Maybe: | |
lib/Transforms/Utils/ImportedFunctionsInliningStatistics.cpp | ||
105 | As Easwaran mentioned, due to the earlier check/continue, you are guaranteed that NumberOfInlines > 0. So simply change this to a ++. | |
109 | Ditto. | |
140 | s/NewLinefalse/LineEnd/ |
Couple minor comments on the comments, but otherwise looks ok. I'm not convinced the complexity of the new handling in recordInlineto avoid the extra map lookup is necessary since this is just stat counting code not on by default though, but I don't have a strong objection to it either.
include/llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h | ||
---|---|---|
90 | Not so much because iterator is invalidated (although an iterator being invalidated would be a side effect of the same issue) - it is because the DenseMap value may be moved, and you need to store pointers to the InlineGraphNodes elsewhere. IIUC that is. You could just change this to "Unique pointer to InlineGraphNode used since the node pointers are also saved in the InlinedCallees vector." | |
92 | Might want to note that this is ok because you don't dereference the key (using the NameMap to avoid needing to). |
I trieid different code with insert/emplace etc, and it was even uglier. The 2 step indirection is the bad guy here, but at least 1 lookup and 1 allocation is the good guy
include/llvm/Transforms/IPO/InlinerPass.h | ||
---|---|---|
71–72 | Formatting - the empty line has been removed after this. | |
include/llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h | ||
90 | The comment is not very clear to me. What does the value type got to with invalidation? | |
98 | I'm missing something here. Why not make the function name the key to the other map and remove this NameMap? This will change NonImportedCallers as well... | |
lib/Transforms/Utils/ImportedFunctionsInliningStatistics.cpp | ||
31 | I find this quite difficult to read. I prefer using if(...) here instead of ternary operator. Also, you don't need to make ValueLookup a pointer to mapped_type. Keeping it a reference will remove one * operator's use. Also add a comment saying that the second lookup could result in the resizing map and that's why you can't just keep a reference to the uniquq_ptr returned by the lookup. |
lib/Transforms/Utils/ImportedFunctionsInliningStatistics.cpp | ||
---|---|---|
32 | you were right. It's funny that I didn't thought about the solution that is right now. It is even better becase the functionname and Imported is set only once |
include/llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h | ||
---|---|---|
91 | Is is more clear right now? (line 93) |
Like the new handling for inserting the Nodes, looks much cleaner.
include/llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h | ||
---|---|---|
64 | Instead, I like Easwaran's suggestion to make NodesMap a StringMap. Then NonImportedCallers should be a vector of strings. Note that StringMap stores a copy of the key when you insert, so you could store the resulting StringRef (returned when the StringMap entry is inserted) in the NonImportedCallers vector instead of a std::string copy. It is cleaner than using keys that are pointers to possibly deleted memory. | |
96 | s/Ff/If/ | |
lib/Transforms/Utils/ImportedFunctionsInliningStatistics.cpp | ||
62 | Good catch. Call this from the ImportedFunctionsInliningStatistics constructor and save the results there instead. |
include/llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h | ||
---|---|---|
64 | I talked with Easwaran and I don't think there is a point changit the map. It will be much slower and won't get any new information, and it will take a time to change everything again. |
include/llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h | ||
---|---|---|
64 | It's an issue of robustness rather than getting more information. I mentioned earlier too that it seems brittle to use pointers to deleted functions as keys - the code may not try to dereference them now, but someone else may come along and try to do something with the key, dereferencing the pointer to get some additional info, not realizing that it is possibly deallocated. If Easwaran is ok with the code as is, then I'm ok with this for now, but something to clean up later. Please add a FIXME? |
include/llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h | ||
---|---|---|
64 |
Sorry, it appears I did not raise that in the review (think I just asked about it in our chat). In any case, I'd prefer taking a bit more time and making it more robust to avoid the type of future error I described, but I am ok with doing that as a follow-on. |
include/llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h | ||
---|---|---|
65 | I will fix it in next patch. I want to push it right now because I don't want to deal with conflicts later. |
Hi,
This commit caused build failure in Visual Studio 2013:
55>C:\p4\stg_win60\git-llvm-tot\llvm\include\llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h(51): error C2610: 'llvm::ImportedFunctionsInliningStatistics::InlineGraphNode::InlineGraphNode(llvm::ImportedFunctionsInliningStatistics::InlineGraphNode &&)' : is not a special member function which can be defaulted
55>C:\p4\stg_win60\git-llvm-tot\llvm\include\llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h(52): error C2610: 'llvm::ImportedFunctionsInliningStatistics::InlineGraphNode &llvm::ImportedFunctionsInliningStatistics::InlineGraphNode::operator =(llvm::ImportedFunctionsInliningStatistics::InlineGraphNode &&)' : is not a special member function which can be defaulted
59> CGOpenMPRuntime.cpp
58> clangBasic.vcxproj -> C:\p4\stg_win60\git-llvm-tot\build\Debug\lib\clangBasic.lib
56>C:\p4\stg_win60\git-llvm-tot\llvm\include\llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h(51): error C2610: 'llvm::ImportedFunctionsInliningStatistics::InlineGraphNode::InlineGraphNode(llvm::ImportedFunctionsInliningStatistics::InlineGraphNode &&)' : is not a special member function which can be defaulted (C:\p4\stg_win60\git-llvm-tot\llvm\lib\Transforms\IPO\InlineSimple.cpp)
56>C:\p4\stg_win60\git-llvm-tot\llvm\include\llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h(52): error C2610: 'llvm::ImportedFunctionsInliningStatistics::InlineGraphNode &llvm::ImportedFunctionsInliningStatistics::InlineGraphNode::operator =(llvm::ImportedFunctionsInliningStatistics::InlineGraphNode &&)' : is not a special member function which can be defaulted (C:\p4\stg_win60\git-llvm-tot\llvm\lib\Transforms\IPO\InlineSimple.cpp)
56>C:\p4\stg_win60\git-llvm-tot\llvm\include\llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h(51): error C2610: 'llvm::ImportedFunctionsInliningStatistics::InlineGraphNode::InlineGraphNode(llvm::ImportedFunctionsInliningStatistics::InlineGraphNode &&)' : is not a special member function which can be defaulted (C:\p4\stg_win60\git-llvm-tot\llvm\lib\Transforms\IPO\InlineAlways.cpp)
56>C:\p4\stg_win60\git-llvm-tot\llvm\include\llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h(52): error C2610: 'llvm::ImportedFunctionsInliningStatistics::InlineGraphNode &llvm::ImportedFunctionsInliningStatistics::InlineGraphNode::operator =(llvm::ImportedFunctionsInliningStatistics::InlineGraphNode &&)' : is not a special member function which can be defaulted (C:\p4\stg_win60\git-llvm-tot\llvm\lib\Transforms\IPO\InlineAlways.cpp)
56>C:\p4\stg_win60\git-llvm-tot\llvm\include\llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h(51): error C2610: 'llvm::ImportedFunctionsInliningStatistics::InlineGraphNode::InlineGraphNode(llvm::ImportedFunctionsInliningStatistics::InlineGraphNode &&)' : is not a special member function which can be defaulted (C:\p4\stg_win60\git-llvm-tot\llvm\lib\Transforms\IPO\Inliner.cpp)
56>C:\p4\stg_win60\git-llvm-tot\llvm\include\llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h(52): error C2610: 'llvm::ImportedFunctionsInliningStatistics::InlineGraphNode &llvm::ImportedFunctionsInliningStatistics::InlineGraphNode::operator =(llvm::ImportedFunctionsInliningStatistics::InlineGraphNode &&)' : is not a special member function which can be defaulted (C:\p4\stg_win60\git-llvm-tot\llvm\lib\Transforms\IPO\Inliner.cpp)
Formatting - the empty line has been removed after this.