Page MenuHomePhabricator

mtrofin (Mircea Trofin)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 10 2015, 11:03 PM (281 w, 5 d)

Recent Activity

Today

mtrofin added inline comments to D82817: [llvm] Native size estimator for training -Oz inliner.
Mon, Jul 6, 9:42 AM · Restricted Project
mtrofin updated the diff for D82817: [llvm] Native size estimator for training -Oz inliner.

feedback

Mon, Jul 6, 9:42 AM · Restricted Project

Wed, Jul 1

mtrofin added a comment to D82817: [llvm] Native size estimator for training -Oz inliner.

A high level question: is partial reward based training something needed in the long run (as compared with total reward based pipeline)?

Wed, Jul 1, 2:04 PM · Restricted Project
mtrofin updated the diff for D82817: [llvm] Native size estimator for training -Oz inliner.

lint

Wed, Jul 1, 12:59 PM · Restricted Project

Tue, Jun 30

mtrofin committed rZORG8721195e10b4: [buildbot] add CMAKE_INSTALL_RPATH_USE_LINK_PATH for ML development mode (authored by mtrofin).
[buildbot] add CMAKE_INSTALL_RPATH_USE_LINK_PATH for ML development mode
Tue, Jun 30, 1:36 PM

Mon, Jun 29

mtrofin updated the diff for D82817: [llvm] Native size estimator for training -Oz inliner.

removed duplicate file, one more test

Mon, Jun 29, 3:50 PM · Restricted Project
mtrofin created D82817: [llvm] Native size estimator for training -Oz inliner.
Mon, Jun 29, 3:32 PM · Restricted Project
mtrofin committed rG5918d49ac152: [llvm][NFC] Use llvm_canonicalize_cmake_booleans for LLVM_HAVE_TF_AOT (authored by mtrofin).
[llvm][NFC] Use llvm_canonicalize_cmake_booleans for LLVM_HAVE_TF_AOT
Mon, Jun 29, 12:30 PM
mtrofin closed D82776: [llvm][NFC] Use llvm_canonicalize_cmake_booleans for LLVM_HAVE_TF_AOT.
Mon, Jun 29, 12:29 PM · Restricted Project
mtrofin added inline comments to D81515: [llvm] Release-mode ML InlineAdvisor.
Mon, Jun 29, 9:10 AM · Restricted Project
mtrofin created D82776: [llvm][NFC] Use llvm_canonicalize_cmake_booleans for LLVM_HAVE_TF_AOT.
Mon, Jun 29, 9:10 AM · Restricted Project

Fri, Jun 26

mtrofin committed rZORG4eca29985fc0: [buildbot] ML Bots: fixed LLVM_CCACHE_BUILD (was _CACHE_) (authored by mtrofin).
[buildbot] ML Bots: fixed LLVM_CCACHE_BUILD (was _CACHE_)
Fri, Jun 26, 8:53 PM

Thu, Jun 25

mtrofin accepted D82521: Refactor FunctionPropertiesAnalysis .

lgtm

Thu, Jun 25, 11:20 AM · Restricted Project
mtrofin accepted D82523: Add a Printer to the FunctionPropertiesAnalysis.

lgtm

Thu, Jun 25, 11:20 AM · Restricted Project
mtrofin added inline comments to D82521: Refactor FunctionPropertiesAnalysis .
Thu, Jun 25, 8:33 AM · Restricted Project

Wed, Jun 24

mtrofin added a comment to D82270: Added a Printer to the FunctionPropertiesAnalysis [Obsolete].

What's the value of having the result type have the analyze method? Is there a scenario where we want to get the result and then re-analyze it?

In the new-PM if you preserve the analysis it doesn't need to be recomputed, I thought. Or maybe I misunderstand your question. Could you elaborate?

[EDIT: Do you mean the ...Info struct? It is used by two passes, right?]

What I mean was, why not let the computation happen in the Analysis::run, and the Info is basically an object not meant to be mutated. This is because the PM will call ::run to recompute, and we don't really have (or want to have) a scenario where we want someone to call Info::analyze() (unless I'm missing something)

Oh, I see. This is a common pattern, I think. Mostly because it allows the passes to be minimal wrappers. This is good as we might want to have an old-PM pass as well. Then you want the analyze in the info "for sure". Even without an old-PM pass, this way allows old-PM cgscc passes to create the info object and populate it, something that doesn't work in the old-PM through the pass mechanism.

Is there a strong reason or is it more a design preference?

Sounds like we need the Info's self-refreshing ability to support old-PM scenarios, correct? If I understand it correctly, there, the Info object could be retrieved from the analysis (this'd be an old-PM analysis, to be added), and then the client could refresh it without the involvement of the analysis or analysis manager, but updating the state the Analysis manager. Is that even desirable?

Its for the old-PM scenario but there is no need for the result to be self-refreshing. The key point is that the analyze method is shared, thus not part of the new-PM pass. If we want a immutable result we can again create a Result class in the FunctionPropertiesInfo. I don't mind that either way as long as analyze (and print) logic is outside of FunctionPropertiesAnalysis I'm happy. Does that make sense?

could it be shared by having it as a static (basically, factory method) on FunctionPropertiesInfo?

sure.

Wed, Jun 24, 2:40 PM · Restricted Project
mtrofin added a comment to D81765: Don't inline dynamic allocas that simplify to huge static allocas..

Not sure I follow this in the patch description:
"Avoid inlining functions if this would result, as even if we didn't hoist the alloca it would remain an dynamic alloca in the caller body."

What would be different between: inlining and not hoisting the alloca, or not inlining?

That's a poor description on my part, you can disregard it. If we inlined it but didn't hoist the alloca from the never-executed block, it would still cause the function to have a dynamic alloca present and perhaps negatively impact codegen as Eli alluded to earlier in this thread.

Wed, Jun 24, 2:40 PM · Restricted Project
mtrofin added a comment to D82270: Added a Printer to the FunctionPropertiesAnalysis [Obsolete].

Sounds like we need the Info's self-refreshing ability to support old-PM scenarios, correct? If I understand it correctly, there, the Info object could be retrieved from the analysis (this'd be an old-PM analysis, to be added), and then the client could refresh it without the involvement of the analysis or analysis manager, but updating the state the Analysis manager. Is that even desirable?

Its for the old-PM scenario but there is no need for the result to be self-refreshing. The key point is that the analyze method is shared, thus not part of the new-PM pass. If we want a immutable result we can again create a Result class in the FunctionPropertiesInfo. I don't mind that either way as long as analyze (and print) logic is outside of FunctionPropertiesAnalysis I'm happy. Does that make sense?

Wed, Jun 24, 1:34 PM · Restricted Project
mtrofin committed rZORGd99cf8c14b7b: [buildbot] Use paths instead of env vars for ml bots (authored by mtrofin).
[buildbot] Use paths instead of env vars for ml bots
Wed, Jun 24, 1:02 PM
mtrofin closed D82482: [buildbot] Use paths instead of env vars for ml bots.
Wed, Jun 24, 1:02 PM
mtrofin added a comment to D82482: [buildbot] Use paths instead of env vars for ml bots.

ok as a workaround as there is precedence using hardcoded paths in the same file.

Wed, Jun 24, 1:01 PM
mtrofin added a comment to D81765: Don't inline dynamic allocas that simplify to huge static allocas..

Not sure I follow this in the patch description:
"Avoid inlining functions if this would result, as even if we didn't hoist the alloca it would remain an dynamic alloca in the caller body."

Wed, Jun 24, 12:28 PM · Restricted Project
mtrofin accepted D82205: InlineCost - method ::print() to allow dump of statistics to non-debug builds.

lgtm

Wed, Jun 24, 12:28 PM · Restricted Project
mtrofin added inline comments to D77752: [llvm] Machine Learned policy for inlining -Oz.
Wed, Jun 24, 11:55 AM · Restricted Project
mtrofin added inline comments to D81515: [llvm] Release-mode ML InlineAdvisor.
Wed, Jun 24, 11:21 AM · Restricted Project
mtrofin created D82482: [buildbot] Use paths instead of env vars for ml bots.
Wed, Jun 24, 10:47 AM
mtrofin added a reverting change for rG62841415e685: [llvm] Added support for stand-alone cmake object libraries.: rG6a890885237a: Revert "[llvm] Added support for stand-alone cmake object libraries.".
Wed, Jun 24, 9:46 AM
mtrofin committed rG6a890885237a: Revert "[llvm] Added support for stand-alone cmake object libraries." (authored by mtrofin).
Revert "[llvm] Added support for stand-alone cmake object libraries."
Wed, Jun 24, 9:46 AM
mtrofin added a comment to D82270: Added a Printer to the FunctionPropertiesAnalysis [Obsolete].

What's the value of having the result type have the analyze method? Is there a scenario where we want to get the result and then re-analyze it?

In the new-PM if you preserve the analysis it doesn't need to be recomputed, I thought. Or maybe I misunderstand your question. Could you elaborate?

[EDIT: Do you mean the ...Info struct? It is used by two passes, right?]

What I mean was, why not let the computation happen in the Analysis::run, and the Info is basically an object not meant to be mutated. This is because the PM will call ::run to recompute, and we don't really have (or want to have) a scenario where we want someone to call Info::analyze() (unless I'm missing something)

Oh, I see. This is a common pattern, I think. Mostly because it allows the passes to be minimal wrappers. This is good as we might want to have an old-PM pass as well. Then you want the analyze in the info "for sure". Even without an old-PM pass, this way allows old-PM cgscc passes to create the info object and populate it, something that doesn't work in the old-PM through the pass mechanism.

Is there a strong reason or is it more a design preference?

Wed, Jun 24, 9:42 AM · Restricted Project
mtrofin committed rGbdceefe95ba6: [llvm] Release-mode ML InlineAdvisor (authored by mtrofin).
[llvm] Release-mode ML InlineAdvisor
Wed, Jun 24, 8:37 AM
mtrofin committed rG62841415e685: [llvm] Added support for stand-alone cmake object libraries. (authored by mtrofin).
[llvm] Added support for stand-alone cmake object libraries.
Wed, Jun 24, 8:37 AM
mtrofin closed D81515: [llvm] Release-mode ML InlineAdvisor.
Wed, Jun 24, 8:37 AM · Restricted Project
mtrofin added inline comments to D77752: [llvm] Machine Learned policy for inlining -Oz.
Wed, Jun 24, 8:04 AM · Restricted Project

Tue, Jun 23

mtrofin added inline comments to D81765: Don't inline dynamic allocas that simplify to huge static allocas..
Tue, Jun 23, 9:35 PM · Restricted Project
mtrofin added inline comments to D81765: Don't inline dynamic allocas that simplify to huge static allocas..
Tue, Jun 23, 7:59 PM · Restricted Project
mtrofin added a comment to D82270: Added a Printer to the FunctionPropertiesAnalysis [Obsolete].

What's the value of having the result type have the analyze method? Is there a scenario where we want to get the result and then re-analyze it?

In the new-PM if you preserve the analysis it doesn't need to be recomputed, I thought. Or maybe I misunderstand your question. Could you elaborate?

[EDIT: Do you mean the ...Info struct? It is used by two passes, right?]

Tue, Jun 23, 2:31 PM · Restricted Project
mtrofin added inline comments to D81765: Don't inline dynamic allocas that simplify to huge static allocas..
Tue, Jun 23, 12:21 PM · Restricted Project
mtrofin added inline comments to D81765: Don't inline dynamic allocas that simplify to huge static allocas..
Tue, Jun 23, 7:26 AM · Restricted Project

Mon, Jun 22

mtrofin added inline comments to D82205: InlineCost - method ::print() to allow dump of statistics to non-debug builds.
Mon, Jun 22, 11:49 AM · Restricted Project
mtrofin updated the diff for D81515: [llvm] Release-mode ML InlineAdvisor.

clang-tidy

Mon, Jun 22, 10:44 AM · Restricted Project
mtrofin added a comment to D82270: Added a Printer to the FunctionPropertiesAnalysis [Obsolete].

What's the value of having the result type have the analyze method? Is there a scenario where we want to get the result and then re-analyze it?

Mon, Jun 22, 9:07 AM · Restricted Project
mtrofin accepted D82283: Add new function properties to FunctionPropertiesAnalysis.

lgtm

Mon, Jun 22, 8:35 AM · Restricted Project

Fri, Jun 19

mtrofin added a comment to D82205: InlineCost - method ::print() to allow dump of statistics to non-debug builds.

Answering @mtrofin :
Although what you suggested is a valid solution, it will go against the point of this whole pass - to enable the checks of inliner's inner works for all builds, not just the debug ones. If we apply your solution, then still we will get several builds which will avoid being checked due to the conditioning flags.

Fri, Jun 19, 2:41 PM · Restricted Project
mtrofin added a comment to D82205: InlineCost - method ::print() to allow dump of statistics to non-debug builds.

I believe the reason dump() is hidden behind conditional compilation flags, is to avoid the size overhead in the respective builds.

Fri, Jun 19, 12:31 PM · Restricted Project
mtrofin updated the diff for D81515: [llvm] Release-mode ML InlineAdvisor.

more details in test

Fri, Jun 19, 12:31 PM · Restricted Project
mtrofin added inline comments to D81515: [llvm] Release-mode ML InlineAdvisor.
Fri, Jun 19, 12:31 PM · Restricted Project

Thu, Jun 18

mtrofin updated the diff for D81515: [llvm] Release-mode ML InlineAdvisor.

fix some formatting

Thu, Jun 18, 11:27 AM · Restricted Project
mtrofin accepted D81716: Extend InlineFeatureAnalysis to more extract generic code features [Obsolete].

lgtm

Thu, Jun 18, 9:45 AM · Restricted Project
mtrofin added inline comments to D81515: [llvm] Release-mode ML InlineAdvisor.
Thu, Jun 18, 7:02 AM · Restricted Project
mtrofin updated the diff for D81515: [llvm] Release-mode ML InlineAdvisor.

correct triple

Thu, Jun 18, 7:02 AM · Restricted Project

Wed, Jun 17

mtrofin added inline comments to D81515: [llvm] Release-mode ML InlineAdvisor.
Wed, Jun 17, 5:17 PM · Restricted Project
mtrofin updated the diff for D81515: [llvm] Release-mode ML InlineAdvisor.

target triple

Wed, Jun 17, 5:17 PM · Restricted Project
mtrofin accepted D82044: Rename InlineFeatureAnalysis to FunctionPropertiesAnalysis.

Nit: I think the motivation was that we want to experiment with extending it and purposing it for other than inliner usecases (as opposed to avoiding confusion - it's not clear what confusion there was)

Wed, Jun 17, 2:02 PM · Restricted Project
mtrofin added inline comments to D81515: [llvm] Release-mode ML InlineAdvisor.
Wed, Jun 17, 2:02 PM · Restricted Project
mtrofin updated the diff for D81515: [llvm] Release-mode ML InlineAdvisor.

Feedback

Wed, Jun 17, 2:02 PM · Restricted Project

Tue, Jun 16

mtrofin accepted D81024: InlineCostAnnotationPrinterPass - print constants to which instructions are simplified.

lgtm

Tue, Jun 16, 3:23 PM · Restricted Project
mtrofin accepted D81743: InlineCostAnnotationPrinterPass - Introducing the pass.

1 more nit - in the description "given one and them dumps the result": s/them/then

Tue, Jun 16, 3:23 PM · Restricted Project
mtrofin added a comment to D81716: Extend InlineFeatureAnalysis to more extract generic code features [Obsolete].

The printer pass needs a test. @mtrofin WDYT?

Yup - never hurts to add a test.

My comment was formatted badly. The test is needed for sure, wanted to know if you are fine with the rename ;)

Tue, Jun 16, 12:39 PM · Restricted Project
mtrofin added a comment to D81716: Extend InlineFeatureAnalysis to more extract generic code features [Obsolete].

The printer pass needs a test. @mtrofin WDYT?

Tue, Jun 16, 11:32 AM · Restricted Project
mtrofin added a comment to D81743: InlineCostAnnotationPrinterPass - Introducing the pass.
  • Refactored the comment For now, my view of the pass is to be able to tell what optimizations are seen and predicted by InlineCost. E.g. is a follow-up patch D81024 which enables printing of constants to which the instruction will be simplified. For now, if you've written a simplification for InlineCost that constant-folds some sequence of instructions, you have no way of writing test for it except by setting the inline threshold so low, so the fact of simplification will inline the method. This pass provides an easy way to see such cases directly. Also, the fact of simplification itself is independent of InlineParams, so the pass would work as expected.
Tue, Jun 16, 9:21 AM · Restricted Project

Mon, Jun 15

mtrofin committed rG296e47734e62: [llvm][NFC] Fix license on InlineFeaturesAnalysis.{h|cpp} (authored by mtrofin).
[llvm][NFC] Fix license on InlineFeaturesAnalysis.{h|cpp}
Mon, Jun 15, 7:52 PM
mtrofin closed D81896: [llvm][NFC] Fix license on InlineFeaturesAnalysis.{h|cpp}.
Mon, Jun 15, 7:51 PM · Restricted Project
mtrofin accepted D81026: Inline Cost improvement - GetElementPtr with constant operands.
  • Added flag for the change which is true by default I have been struggling to collect the data @mtrofin has asked for to prove the usefulness of the patch. I will continue to do so, but meanwhile, I suggest accepting the change. Once I have gathered needed data, I will post new differential presenting the results and (most likely) deleting the flag.
Mon, Jun 15, 7:51 PM · Restricted Project
mtrofin updated the diff for D81515: [llvm] Release-mode ML InlineAdvisor.

Default TENSORFLOW_AOT_PATH should be "", not its description.

Mon, Jun 15, 6:14 PM · Restricted Project
mtrofin updated the diff for D81515: [llvm] Release-mode ML InlineAdvisor.

Moved everything to Analysis

Mon, Jun 15, 5:42 PM · Restricted Project
mtrofin added inline comments to D81515: [llvm] Release-mode ML InlineAdvisor.
Mon, Jun 15, 5:42 PM · Restricted Project
mtrofin added inline comments to D80579: [llvm] Add function feature extraction analysis.
Mon, Jun 15, 5:08 PM · Restricted Project
mtrofin created D81896: [llvm][NFC] Fix license on InlineFeaturesAnalysis.{h|cpp}.
Mon, Jun 15, 5:08 PM · Restricted Project
mtrofin added inline comments to D80579: [llvm] Add function feature extraction analysis.
Mon, Jun 15, 5:08 PM · Restricted Project
mtrofin closed D81447: [llvm][NFC] Move content of ML subdirectory into Analysis.
Mon, Jun 15, 3:29 PM · Restricted Project
mtrofin added 1 commit(s) for D81447: [llvm][NFC] Move content of ML subdirectory into Analysis: rGe2cc854015fe: [llvm][NFC] Move content of ML subdirectory into Analysis.
Mon, Jun 15, 3:29 PM · Restricted Project
mtrofin added an edge to rGe2cc854015fe: [llvm][NFC] Move content of ML subdirectory into Analysis: D81447: [llvm][NFC] Move content of ML subdirectory into Analysis.
Mon, Jun 15, 3:29 PM
mtrofin committed rGe2cc854015fe: [llvm][NFC] Move content of ML subdirectory into Analysis (authored by mtrofin).
[llvm][NFC] Move content of ML subdirectory into Analysis
Mon, Jun 15, 2:57 PM
mtrofin added a comment to D81716: Extend InlineFeatureAnalysis to more extract generic code features [Obsolete].

@mtrofin We want to "rename" InlineFeatureAnalysis to a more generic name and extend it. This patch does the former, basically, and adds a printer pass. Extensions will follow soon. Is that generally OK with you?

@tarinduj I left some comments. We also need to replace the old InlineFeaturesAnalsysis with the CodeFeature one everywhere, assuming @mtrofin doesn't have any concerns. Generally we minimize duplication ;)

Some feature calculations are computationally intensive. At least for inlining, we plan in a next step to look at regions of the call graph around a call site. I wouldn't want to burden other consumers with this extra cost.

That is fair, see below.

As a first step, if the planned new features would add space or computational overhead, could the work requiring the extra features have its own analysis and consume both analysis results? As we have examples of more such analyses and understand the usecases and performance tradeoffs, we can very easily refactor / rename.

While we can have more analysis if we want, I somehow doubt that will make it better. I was envisioning a lazy approach in which the analysis will not do anything until instructed to. Users can "ask" for what they want while reuse is still possible and code is kept at a single place. As an example, if users ask to findCFGstructureFeatures they will not get any instruction level results. Querying those could assert w/o extra cost. I don't see why this would cost us extra and I would prefer it very much over a multitude of passes. WDYT?

Mon, Jun 15, 2:54 PM · Restricted Project
mtrofin retitled D81447: [llvm][NFC] Move content of ML subdirectory into Analysis from [llvm] Added support for stand-alone cmake object libraries. to [llvm][NFC] Move content of ML subdirectory into Analysis.
Mon, Jun 15, 1:46 PM · Restricted Project
mtrofin updated the diff for D81447: [llvm][NFC] Move content of ML subdirectory into Analysis.

Alternative

Mon, Jun 15, 1:46 PM · Restricted Project
mtrofin reopened D81447: [llvm][NFC] Move content of ML subdirectory into Analysis.
Mon, Jun 15, 1:46 PM · Restricted Project
mtrofin added 1 commit(s) for D81447: [llvm][NFC] Move content of ML subdirectory into Analysis: rG29e572294977: Revert "[llvm] Added support for stand-alone cmake object libraries.".
Mon, Jun 15, 1:19 PM · Restricted Project
mtrofin added an edge to rG29e572294977: Revert "[llvm] Added support for stand-alone cmake object libraries.": D81447: [llvm][NFC] Move content of ML subdirectory into Analysis.
Mon, Jun 15, 1:19 PM
mtrofin added a comment to D81447: [llvm][NFC] Move content of ML subdirectory into Analysis.

It is kinda bad form to merge a change that has unaddressed feedback.

Mon, Jun 15, 1:14 PM · Restricted Project
mtrofin committed rG29e572294977: Revert "[llvm] Added support for stand-alone cmake object libraries." (authored by mtrofin).
Revert "[llvm] Added support for stand-alone cmake object libraries."
Mon, Jun 15, 12:41 PM
mtrofin added a reverting change for rG695c7d6313d7: [llvm] Added support for stand-alone cmake object libraries.: rG29e572294977: Revert "[llvm] Added support for stand-alone cmake object libraries.".
Mon, Jun 15, 12:41 PM
mtrofin added a comment to D81447: [llvm][NFC] Move content of ML subdirectory into Analysis.
Mon, Jun 15, 12:37 PM · Restricted Project
mtrofin committed rG695c7d6313d7: [llvm] Added support for stand-alone cmake object libraries. (authored by mtrofin).
[llvm] Added support for stand-alone cmake object libraries.
Mon, Jun 15, 12:08 PM
mtrofin closed D81447: [llvm][NFC] Move content of ML subdirectory into Analysis.
Mon, Jun 15, 12:08 PM · Restricted Project
mtrofin added inline comments to D66374: [SampleFDO] Add symbol whitelist in the profile and use it when profile-sample-accurate is enabled.
Mon, Jun 15, 9:13 AM · Restricted Project
mtrofin added a comment to D81716: Extend InlineFeatureAnalysis to more extract generic code features [Obsolete].

@mtrofin We want to "rename" InlineFeatureAnalysis to a more generic name and extend it. This patch does the former, basically, and adds a printer pass. Extensions will follow soon. Is that generally OK with you?

@tarinduj I left some comments. We also need to replace the old InlineFeaturesAnalsysis with the CodeFeature one everywhere, assuming @mtrofin doesn't have any concerns. Generally we minimize duplication ;)

Mon, Jun 15, 7:33 AM · Restricted Project
mtrofin added a comment to D81447: [llvm][NFC] Move content of ML subdirectory into Analysis.

Gentle reminder - thank you!

Mon, Jun 15, 7:33 AM · Restricted Project

Fri, Jun 12

mtrofin added a comment to D81743: InlineCostAnnotationPrinterPass - Introducing the pass.

Changed the creation of InlineParams back to default params and added a "to-do" comment.
For the purposes of this pass, the idea of which is to give access for non-debug builds to tests that use debug functionality, the InlineParams can be taken as default. My suggestion is to leave the extension of the pass to parametrization with different InlineParams as a "to-do" for now.

Fri, Jun 12, 6:15 PM · Restricted Project
mtrofin added a comment to D81765: Don't inline dynamic allocas that simplify to huge static allocas..

What happens if the alloca that is left in the conditional instead of being moved at the caller's entry block is on the hot path, like in a loop?

Fri, Jun 12, 5:47 PM · Restricted Project
mtrofin updated the diff for D81515: [llvm] Release-mode ML InlineAdvisor.

rebased + feedback

Fri, Jun 12, 9:45 AM · Restricted Project
mtrofin added a comment to D81743: InlineCostAnnotationPrinterPass - Introducing the pass.

A question to @mtrofin:
I have changed the creation of InlineParams from

const InlineParams Params = llvm::getInlineParams();

which invokes the creation of InlineParams with DefaultThreshold to

const InlineParams Params = llvm::getInlineParams(InlineThreshold);

Is this correct?

Fri, Jun 12, 8:39 AM · Restricted Project

Thu, Jun 11

mtrofin added a comment to D81447: [llvm][NFC] Move content of ML subdirectory into Analysis.

Is this good to go?

Thu, Jun 11, 3:59 PM · Restricted Project
mtrofin accepted D81687: InlineCostAnnotationPrinterPass - preparational patch. Renaming structs, renaming flag, refactoring.

lgtm

Thu, Jun 11, 2:19 PM · Restricted Project
mtrofin added inline comments to D81223: Size LTO (1/3): Standardizing the use of OptimizationLevel .
Thu, Jun 11, 9:19 AM · Restricted Project, Restricted Project, lld, Restricted Project
mtrofin committed rGe82eff7a03b0: [llvm][NFC] Factor some common data in InlineAdvice (authored by mtrofin).
[llvm][NFC] Factor some common data in InlineAdvice
Thu, Jun 11, 8:14 AM
mtrofin closed D81507: [llvm][NFC] Factor some common data in InlineAdvice.
Thu, Jun 11, 8:14 AM · Restricted Project

Wed, Jun 10

mtrofin added inline comments to D81507: [llvm][NFC] Factor some common data in InlineAdvice.
Wed, Jun 10, 4:41 PM · Restricted Project
mtrofin added a comment to D81515: [llvm] Release-mode ML InlineAdvisor.

If there's some standardized binary format for models, that's might be okay? By analogy, there are some PNG files in the documentation; we don't insist people use XPM or something like that. There are some technical reasons to prefer text, though: it would allow someone to identify or diff the contents of the files without specialized tools.

Wed, Jun 10, 4:08 PM · Restricted Project
mtrofin added a comment to D81016: Adding InlineCostAnnotationPrinterPass for Inline Cost Analysis.

Ping

Wed, Jun 10, 2:28 PM · Restricted Project