Page MenuHomePhabricator

GalZohar (Gal Zohar)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 31 2020, 12:01 PM (30 w, 3 d)

Recent Activity

Jan 12 2021

GalZohar added a comment to D43256: [MBP] Move a latch block with conditional exit and multi predecessors to top of loop.

@GalZohar, thanks for the example, now I understand your problem. The new layout does increase the number of executed branches for the path BB2->BB3->BB4->BB2. Unfortunately most of the current MBP algorithms don't consider this factor. When considering the number of fall through only, the new layout has more fall through and less taken branch.

Since the number of executed branches does has performance impact on your target, so welcome to enhance MBP to include this factor on related targets.

Jan 12 2021, 11:55 PM · Restricted Project

Jan 8 2021

GalZohar added a comment to D43256: [MBP] Move a latch block with conditional exit and multi predecessors to top of loop.

@GalZohar, without a testcase I can't say what's the problem.

It is not intentional to increase the number of branches of any particular path. All of the algorithms are driven by branch probabilities. If you didn't collect profiling, llvm guesses a probability for each branch, it's reasonable for most code. But it's not rare that the guessed probability is different from actual probability, it may not result in a good layout.

So I strongly suggest you try profiling build. MBP is one of the passes that get most performance improvement from profiling.

Jan 8 2021, 11:51 AM · Restricted Project

Jan 7 2021

GalZohar added a comment to D43256: [MBP] Move a latch block with conditional exit and multi predecessors to top of loop.

@GalZohar, the various layout algorithms in MachineBlockPlacement mainly consider the number of fall-throughs and dynamic number of branch instructions (usually they are consistent) according to branch probabilities. So you can try to build your application with profiling. Or you can compile this file with -Os since the improvement in this patch is disabled with -Os.

Jan 7 2021, 1:15 PM · Restricted Project

Dec 31 2020

GalZohar added a comment to D43256: [MBP] Move a latch block with conditional exit and multi predecessors to top of loop.

Hi,
Seems like this is supposed to increase the average number of fallthrough in a case of simple nested loops, however it seems to also increase the total number of branch instructions both in and outside the outer loop.
In our target this seems to do more harm than good as increasing the number of branches is significantly more harmful than having more conditional taken branches.
Instead of a conditional branch that is usually taken and jumps from end to start of outer loop, we have an unconditional branch that jumps from the end of the inner loop to the end of the outer loop, which then conditionally jumps out of the outer loop or falls through to the next iteration of the outer loop.
This increases the total number of executed branch instructions by 1.

Dec 31 2020, 12:19 PM · Restricted Project