- User Since
- Feb 10 2017, 1:36 AM (79 w, 1 d)
NB: this is still at early stages, in particular PassExecutionCounter is just a stub for now.
Yet I need a review for people to tell me if I'm completely off the track! :)
Tue, Aug 14
Mon, Aug 13
Sun, Aug 12
I'm really worried about LoopSafetyInfo getting more nontrivial state
(Philip already raised a question in D50426 about CurLoop potentially becoming invalid, and here we get even more with addition of ICF).
Fri, Aug 3
LGTM bar some minor stylistic stuff.
Thu, Aug 2
Tue, Jul 31
Shouldnt this be reopened?
Any plans to reintegrate this back - it seems to provide a very useful functionality...
Mon, Jul 30
Jul 4 2018
Hmm... downstream testing found an issue.
Here is a reduced testcase (https://reviews.llvm.org/file/data/b4nvnqme23unyxwktst4/PHID-FILE-wny66mgcso2cbh4ciz3x/reduced-unswitch.ll)
Jul 3 2018
Btw, I'm running downstream testing with this set of changes, but since I did not see anything like 37889 before I dont think its gonna find anything here as well.
Ok, makes sense.
LGTM for the code.
Jun 24 2018
As per my latest comment there was an assert when I ran pre-integration testing.
I dont have any immediate plans to continue investigation on that, so if you can take it over from me - please, do.
Functionally this patch was heavily tested as part of our local clang-on-sparc-Solaris installation back at the time when I was part of Oracle Developer Studio project.
Jun 8 2018
Not sure where to put this into... but I ran an experiment by adding -passes=loop(unswitch) to the LoopUnswitch tests and here is the result
with the build of all the three dependent fixes:
Failing Tests (11):
LLVM :: Transforms/LoopUnswitch/2011-11-18-TwoSwitches.ll LLVM :: Transforms/LoopUnswitch/basictest.ll LLVM :: Transforms/LoopUnswitch/copy-metadata.ll LLVM :: Transforms/LoopUnswitch/elseif-non-exponential-behavior.ll LLVM :: Transforms/LoopUnswitch/guards.ll LLVM :: Transforms/LoopUnswitch/infinite-loop.ll LLVM :: Transforms/LoopUnswitch/msan.ll LLVM :: Transforms/LoopUnswitch/simplify-with-nonvalness.ll LLVM :: Transforms/LoopUnswitch/trivial-unswitch.ll LLVM :: Transforms/LoopUnswitch/unswitch-equality-undef.ll LLVM :: Transforms/LoopUnswitch/unswitch-select.ll
Jun 6 2018
Jun 1 2018
May 29 2018
May 28 2018
Not exactly related to this set of changes...
I just have discovered that old LoopUnswitch performs SCEV cache invalidation when it does nontrivial updates to CFG
(forgetLoop in LoopUnswitch::unswitchNontrivialCondition) .
Looks ok to me.
May 24 2018
The pipeline will now be ... truly dynamic.
May 22 2018
Can you, please, describe the reason for this change?
May 11 2018
May 10 2018
What about finally renaming SimpleLoopUnswitch into something "less simple"? :)
Apr 24 2018
Looks good to me.
Thanks for the effort.
This version finally passes all the tests on our internal pipeline!
Apr 19 2018
Ahem... sorry for checking it only now, but this does not seem to address a testcase from PR36379, so it must be a different issue.
I will put this to more testing to see if it improves anything on our workflow.
Apr 5 2018
Apr 4 2018
a few nits, LGTM as soon as those are corrected.
Apr 3 2018
Apr 2 2018
LGTM, but, please, wait for Andrew to ack this.
Mar 30 2018
Mar 27 2018
Mar 26 2018
LGTM. Though, please, wait for Andrew's feedback.
Mar 23 2018
minor semantical update - calculation of return value corrected for AttributeInferer::run,
returning true only when real changes are detected.
Fixes regression introduced by most recent algorithmic update.
Thanks for following up till the very end of it! :)
Your feeling of language is a bit over my capabilities, but I do like to learn here.
minor comments cleanup
Mar 22 2018
more comments cleanup
addressing Justin's comments
trying with erase_if
Mar 21 2018
minor cleanup first
just to clarify - I believe this is ready to go :)
Mar 20 2018
making InstrBreaksNonThrowing a bit more readable, introducing -disable-nounwind-inference option just in case
I have seen this suggestion and honestly I dont see how it could be dropped w/o first replacing every invocation of skipPass with a debug-counter's shouldExecute.
Theoretically debug-counters are at least as powerful as OptBisect, but that stays theoretical until somebody really places them into the code.
For now I only see 5 instances of shouldExecute in a whole LLVM tree.
Mar 19 2018
getting rid of bitvectors
Btw, I tried Justin's version of "simplified" meta-programmed callbacks... and fully functional version of it
(one that performs early bailouts on ScanAttrHere/ValidAttrs) starts approaching Chandler's suggested tuple/tuple-iteration by complexity.
The problem with bailouts is that you cant just fill the whole array with calculated values of all the predicates,
or else you need to extend all your predicates with shortcut-ting semantics.
addressing most of Justin's comments on comments ;)
Mar 16 2018
This suggestion is kinda in line with my first suggestion on generalization - introduce a base class OptPassGate, and derive OptBisect from it; routines that skip the passes should operate with OptPassGate, not with OptBisect directly.
Mar 15 2018
Agreed. And this one thing is - "skip the pass".
So in this sense I do not believe we overload the behavior.
fixing comments, adding struct instead of tuple.
Addressed pretty much everything except of bitvectors and virtual functions,
just to have some ready base for comparison with other solutions.
Mar 14 2018
Btw, we do compile in multiple threads, having separate LLVMContexts for each thread.
For our custom JIT compiler pipeline we want to have a flexible control on pipeline behavior,
which might have a number of different applications. Immediate one is the ability to gracefully
shut down the compilation that takes too long.
Most likely this particular need is completely off the roster of a static compiler though.
Thanks everybody for good comments, will see into them.
Will put virtual functions issue at the very end of the queue.
Mar 13 2018
If anybody has any other comments/objectsions, please, speak up.
I'm going to push this tomorrow.
all the tests updated, one FIXME left on convergent.ll test as intended
cleaning up some tests, still a couple FIXMEs in tests
Mar 12 2018
making BPI completely optional (removing all the LPM changes), cleanup
It appears that turning BPI analysis into the standard one might prove to be rather costly (as that will cause this analysis to be run for every loop pass manager invocation, and currently there are none of the loop passes that use BPI in default pipelines).
I will craft a version of this patch that makes BPI completely optional in IRCE w/o any changes to LoopPassManager.
That will allow to decouple IRCE port from (a rather nonobvious) solution for BPI.
Should this separate BPI patch go before or after this change?
Mar 5 2018
Feb 28 2018
Feb 26 2018
I'm not happy with BPI solution in this patch.
It is mainly to provoke discussions on how to solve this more gracefully, since no answers were given on llvm-dev@ :