User Details
- User Since
- Apr 10 2015, 8:26 AM (313 w, 1 d)
Jul 27 2020
LGTM. How to change NeedRevision flag?
Jul 26 2020
I don't have strong preference for bwd direction. Let's get this fix in. Are all tests and sanities passing?
Jul 25 2020
Thank you for checking on mainMBB and fallBB. Which direction: forward or backward - do you prefer for isEFLAGSLiveAfter()?
Yes, check for mi.killsRegister(X86::EFLAGS)) before a def (in backward order).
The instructions in mainMBB and fallBB during xbegin() expansion do not deal with EFLAGS. If a subsequent pass makes changes in mainMBB and fallBB that might impact the liveness of EFLAGS, that pass should make proper updates. Having
mainMBB->addLiveIn(X86::EFLAGS); fallMBB->addLiveIn(X86::EFLAGS);
is good too.
Jul 24 2020
Hi Craig, thank you for working on this bug.
Jul 18 2019
Where is the Xtensa instruction set architecture (ISA) document on which this patch is based on?
Feb 3 2016
Hi Mitch,
Jan 19 2016
Hi David, Diego, thanks for the comments. Sorry for the delayed response.
Dec 10 2015
Agree, but this depends what tests/apps we are concerned about. LNT tests didn't show many outliers.
In addition to Chromium, which applications have been known as outliers in terms of DSE compile-time contribution in the past?
Hi Chad, thanks for helping with that. Is compile-time increase the remaining problem?
One quick fix could be to change 100 to 10 for the max number on non-local attempts in a function:
static const unsigned MaxNonLocalAttempts = 100;
We've chosen 100 arbitrarily. The newly added DSE tests should pass with 10.
Dec 9 2015
LGTM
Dec 5 2015
Thanks, Chad. Can we commit the patch at this point?
It solves one DSE enhancement known for several years.
I believe the patch has been thoroughly reviewed and tested.
Nov 20 2015
Nov 19 2015
Mitch, I added a relevant comment above DT->dominates(BB, Pred).
Added a comment to clarify avoidance of blocks (to search for redundant stores) that form a cycle with BB, as requested by Mitch.
Update to ToT is not necessary because the recent changes to DeadStoreElimination.cpp has been reverted.
Nov 18 2015
ping?
Nov 17 2015
Could someone with commit access commit this patch, if there is no objection from reviewers?
Nov 16 2015
Hi Bruno,
Nov 15 2015
Any objection to commit this patch?
Nov 12 2015
Added option -enable-nonlocal-dse to enable/disable the feature.
Nov 6 2015
Hi Bruno,
Nov 4 2015
Hi Bruno, the patch has been rebased, and should apply cleanly on ToT.
Rebased.
Address Mitch's comments.
Nov 2 2015
Thanks, Bruno. If necessary we need to balance non-local DSE for compile-time and run-time performance.
Nov 1 2015
addressed two comments by Mitch
Oct 28 2015
Hi Bruno,
Hi Mitch, thanks for the thoughtful comments; we are making a good progress.
I will make the two changes: using successor iterator and adding the comment, in the next revision.
Oct 27 2015
ping?
Oct 24 2015
Oct 22 2015
Oct 20 2015
ping?
Oct 19 2015
changes for style and coding standards
Oct 18 2015
Thanks for suggestions/comments, Mitch. My responses are inlined below.
The new version now handles a redundant store in the if-block in if-then-else - based on a suggestion by Mitch. We use SafeBlocks set, which is initialized and updated in DSE::HandleNonLocalDependency() and used in helper FindSafePreds(). SafeBlocks keeps track of all predecessor blocks that are safe to search through. A new test - ifthenelse.ll - has been added.
Oct 13 2015
ping?
Oct 9 2015
Addressed Erik's comments:
*) Your algorithm has quadratic complexity in worst case. I know this is the same as for the single block algorithm, but extending it to multiple blocks might worsen the situation. Maybe you add some sort of upper bound for the number of dependency checks?
Oct 6 2015
Oct 1 2015
Note that the code for BB DSE stays the same, it is just indented two spaces to the right.
The new code begins at:
// DSE across BB else if (InstDep.isNonLocal()) { ...
and it includes helper FindUncondPredsIncludingSimpleLoops().