- User Since
- Feb 10 2015, 11:03 PM (302 w, 2 d)
Sat, Nov 21
Thu, Nov 19
patched up tests
Scoped changes to just llvm/test/Analysis
Wed, Nov 18
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.
Running just the always inliner variant, without other passes.
gentle reminder - is this OK to land now? Thanks!
Tue, Nov 17
Mon, Nov 16
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)
Sun, Nov 15
Thanks for doing this! LGTM, but please wait for signoff from a x86 owner.
Sat, Nov 14
Thu, Nov 12
Wed, Nov 11
Tue, Nov 10
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.
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).
Maybe the popular RegisterRef can be moved to Register.h? Would that complicate this patch?
lit.local.cfg would "blanket-allow" future tests, too. Would that be
Mon, Nov 9
Thu, Nov 5
Wed, Nov 4
Sorry for the delay - ptal. Thanks!
Tue, Nov 3
Would we rather want them to be i64, or is there a fundamental reason they should be i32?
Mon, Nov 2
Thanks for looking into this!
Fri, Oct 30
Sorry, got a bit eager submitting, as I wanted to unblock uses of the new flag. Happy to address post-submit feedback, if any!
Sorry, hit push very quickly - happy to address post-submit feedback.