[PATCH] D140542: [MachineCombiner] Support local strategy for traces

Mon Feb 6 04:57:33 PST 2023

asi-sc added a comment.

In D140542#4096793 <https://reviews.llvm.org/D140542#4096793>, @spatel wrote:

> The asm diffs in existing test files show that we are affecting the default behavior of RISC-V compiles, right? If those are considered neutral or improvements, then I think this is good to go.

Thanks for the question. Although changes were neutral, I decided we shouldn't change the default behavior. At least not in this change.  So, I slightly updated the patch.

In D140542#4098268 <https://reviews.llvm.org/D140542#4098268>, @Gerolf wrote:

> Given that this is RISC-V and under a flag, this LGTM. I would like to see stats on the FMA's plus the changes to the cycle counts on the critical path, and see how the data correlate to your measured run-time performance numbers.  And ditto for the current heuristic. This might also help understand the wide variety of results in your SPEC data. Your numbers look all over the place. Also, you can probably push your idea more by allowing a parameterized schedule window (eg 10 or 15 instructions) rather than a basic block. This would allow you to catch cases across blocks and should work better for large blocks. Finally, I would not be surprised - just learning from your insights here and guessing - that various in-order processors  show best performance for different window sizes. All this is just food for thought for additional/future work though. Cheers!

Using a schedule window is an interesting idea, thank you. It'll definitely work better at basic block boundaries, however I can imagine basic blocks for which reasonably small schedule window won't work well because of instructions placement before scheduling. It might be a rare corner case though. I'll experiment with this idea at my free time.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140542/new/

https://reviews.llvm.org/D140542