[RISCV] add load/store misched/PostRA cluster options

[danielhb]

Some processors benefit more from store clustering than load clustering,
and vice-versa, depending on factors that are exclusive to each one
(e.g. macrofusions implemented).

Likewise, certain optimizations benefits more from misched clustering
than postRA clustering. Macrofusions are again an example: in a
processor with store pair macrofusions, like the veyron-v1, it is
observed that misched clustering increases the amount of macrofusions
more than postRA clustering. This of course isn't necessarily true for
other processors, but it shows that processors can benefit from a more
fine grained control of clustering mutations, and each one is able to do
it differently.

Add 4 new clustering options that deprecates the existing
riscv-misched-load-store-clustering and
riscv-postmisched-load-store-clustering options:

• riscv-misched-load-clustering and riscv-misched-store-clustering:
  enable/disable load/store clustering during misched;

• riscv-postmisched-load-clustering and
  riscv-postmisched-store-clustering: enable/disable load/store
  clustering during PostRA.

To preserve the existing clustering behavior all 4 options are defaulted to
'true'.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

Author: Daniel Henrique Barboza (danielhb)

Changes

Hi,

This PR adds 4 new cluster options to fine tune how loads and stores are grouped during misched and postRA passes.

The work was motivated by macrofusion tests with the veyron-v1 processor model. This model has stored pair macrofusions but it doesn't do much with load pairs. Adding new options to turn off load clustering improved the macrofusions amount and performance.

The existing 2 cluster options added by @asb are kept because I'm afraid to break existing users that might be using them. If that' s not a concern we can remove them and keep the new options only.

---

Full diff: https://github.com/llvm/llvm-project/pull/149409.diff

1 Files Affected:

• (modified) llvm/lib/Target/RISCV/RISCVTargetMachine.cpp (+34-8)

diff --git a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
index b43b915d0ad4f..24ddd3e1fd1c7 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
@@ -99,11 +99,31 @@ static cl::opt<bool> EnableMISchedLoadStoreClustering(
     cl::desc("Enable load and store clustering in the machine scheduler"),
     cl::init(true));
 
+static cl::opt<bool> EnableMISchedLoadClustering(
+    "riscv-misched-load-clustering", cl::Hidden,
+    cl::desc("Enable load clustering in the machine scheduler"),
+    cl::init(true));
+
+static cl::opt<bool> EnableMISchedStoreClustering(
+    "riscv-misched-store-clustering", cl::Hidden,
+    cl::desc("Enable store clustering in the machine scheduler"),
+    cl::init(true));
+
 static cl::opt<bool> EnablePostMISchedLoadStoreClustering(
     "riscv-postmisched-load-store-clustering", cl::Hidden,
     cl::desc("Enable PostRA load and store clustering in the machine scheduler"),
     cl::init(true));
 
+static cl::opt<bool> EnablePostMISchedLoadClustering(
+    "riscv-postmisched-load-clustering", cl::Hidden,
+    cl::desc("Enable PostRA load clustering in the machine scheduler"),
+    cl::init(true));
+
+static cl::opt<bool> EnablePostMISchedStoreClustering(
+    "riscv-postmisched-store-clustering", cl::Hidden,
+    cl::desc("Enable PostRA store clustering in the machine scheduler"),
+    cl::init(true));
+
 static cl::opt<bool>
     EnableVLOptimizer("riscv-enable-vl-optimizer",
                       cl::desc("Enable the RISC-V VL Optimizer pass"),
@@ -301,10 +321,13 @@ ScheduleDAGInstrs *
 RISCVTargetMachine::createMachineScheduler(MachineSchedContext *C) const {
   ScheduleDAGMILive *DAG = createSchedLive(C);
   if (EnableMISchedLoadStoreClustering) {
-    DAG->addMutation(createLoadClusterDAGMutation(
-        DAG->TII, DAG->TRI, /*ReorderWhileClustering=*/true));
-    DAG->addMutation(createStoreClusterDAGMutation(
-        DAG->TII, DAG->TRI, /*ReorderWhileClustering=*/true));
+    if (EnableMISchedLoadClustering)
+      DAG->addMutation(createLoadClusterDAGMutation(
+          DAG->TII, DAG->TRI, /*ReorderWhileClustering=*/true));
+
+    if (EnableMISchedStoreClustering)
+      DAG->addMutation(createStoreClusterDAGMutation(
+          DAG->TII, DAG->TRI, /*ReorderWhileClustering=*/true));
   }
 
   const RISCVSubtarget &ST = C->MF->getSubtarget<RISCVSubtarget>();
@@ -318,10 +341,13 @@ ScheduleDAGInstrs *
 RISCVTargetMachine::createPostMachineScheduler(MachineSchedContext *C) const {
   ScheduleDAGMI *DAG = createSchedPostRA(C);
   if (EnablePostMISchedLoadStoreClustering) {
-    DAG->addMutation(createLoadClusterDAGMutation(
-        DAG->TII, DAG->TRI, /*ReorderWhileClustering=*/true));
-    DAG->addMutation(createStoreClusterDAGMutation(
-        DAG->TII, DAG->TRI, /*ReorderWhileClustering=*/true));
+    if (EnablePostMISchedLoadClustering)
+      DAG->addMutation(createLoadClusterDAGMutation(
+          DAG->TII, DAG->TRI, /*ReorderWhileClustering=*/true));
+
+    if (EnablePostMISchedStoreClustering)
+      DAG->addMutation(createStoreClusterDAGMutation(
+          DAG->TII, DAG->TRI, /*ReorderWhileClustering=*/true));
   }
 
   return DAG;

[mgudim]

even though the old option is preserved, it now does nothing?

[danielhb]

All added options are defaulted to 'true', similar to the 2 existing options. But the older options have precedence. In the commit msg I gave an example:

"To preserve the existing behavior of the existing flags all options added
are default 'true'. Note that the two existing flags have precedence, so
setting "riscv-misched-load-store-clustering=false" will disable load
mutations even if setting riscv-misched-load-clustering=true. Same thing
with stores and with the postRA options."

So riscv-misched-load-store-clustering=false will turn off misched clustering for both loads and stores, regardless of riscv-misched-(load/store)-clustering being set to 'true' in the same command line.

Perhaps a better wording would be "disabling takes precedence" because that's how the code works, e.g.:

• riscv-misched-load-store-clustering=false will always disable both load and store clustering for misched;
• riscv-misched-load-clustering=false will always disable load clustering for misched;
• riscv-misched-store-clustering=false will always disable store clustering for misched.

I thought about inverting the semantics of the flag, i.e. adding options that would disable steps instead of enabling, but that didn't make the situation much better. What would make it better is to remove the existing 2 options, but then we'll have to deal with potential broken scripts.

[mgudim]

OK, got it. Thanks!

[mgudim]

LGTM, but let's wait to see what others say if it is OK to remove the old options.

[mshockwave]

I'm generally in favor of removing the existing two options (i.e. riscv-misched-load-store-clustering and riscv-postmisched-load-store-clustering). This is not a Clang flag and LLVM flags change all the time.

Non blocking, but an alternative solution to avoid having two flags could be making -riscv-misched-load-store-clustering (and its post-RA counterpart) to take a list of "load" or "store" instead to enable either or both clusterings.

[danielhb]

Thanks for the reviews. Removing the original flags is a cleaner way of going about it so I just did that.

[mshockwave]

LGTM w/ minor comments

Please avoid force push unless necessary: https://llvm.org/docs/GitHub.html#rebasing-pull-requests-and-force-pushes

[mshockwave]

could you add new tests checking the situation where only either of the flags is enabled?

[danielhb]

Sure, I'll add two new tests disabling load and store clustering respectively

[wangpc-pp]

LGTM! Thanks!