[llvm] [RISCV] Add Tune to DontSinkSplatOperands (PR #79199)

Michael Maitland via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 25 07:54:48 PST 2024


================
@@ -1082,6 +1082,13 @@ def TuneShortForwardBranchOpt
 def HasShortForwardBranchOpt : Predicate<"Subtarget->hasShortForwardBranchOpt()">;
 def NoShortForwardBranchOpt : Predicate<"!Subtarget->hasShortForwardBranchOpt()">;
 
+// Some subtargets require a S2V transfer buffer to move scalars into vectors.
+// FIXME: Forming .vx/.vf/.wx/.wf can reduce register pressure.
+def TuneDontSinkSplatOperands
+    : SubtargetFeature<"dont-sink-splat-operands", "DontSinkSplatOperands",
----------------
michaelmaitland wrote:

It looks like `TuneNoDefaultUnroll`, `TuneNoOptimizedZeroStride`, `FeatureNoRVCHints` disable features and contain the `No` string, similar to `Dont`. I don't see any other `SubtargetFeature`s with `false`. Are you sure your suggestion matches with how we disable features?

I made an attempt to implement your suggestion, and here's what I found:

If we had `SinkSplatOperands` and `false`, then we would want to have 
```
if (!Subtarget.sinkSplatOperands())
  return false;
```
since
```
if (Subtarget.sinkSplatOperands())
  return true;
```
is not the desired behavior, since this tuning having a true value does not imply that splat operands should be sunk. Instead, a true value implies that we should allow the rest of the logic in that function to determine whether to sink the splat operands. The desired behavior is that if sinkSplatOperands is false then we will not sink splat operands.

Now lets consider the three command line options:
1. `+sink-splat-operands`
2. `-sink-splat-operands`
3. Tuning not specified

If we had `SinkSplatOperands` and `false`, then only the tuning not specified option works as expected.
If we had `SinkSplatOperands` and `true`, then `+sink-splat-operands` and `-sink-splat-operands` work correctly, but tuning not specified does not sink when it should be sinking.

Am I misunderstanding something? Based on my understanding, I think the tuning is in line with other tunings that try to disable features. The one refinement I can think of is that we can rename to `TuneNoSinkSplatOperands` since the other features use the `No` instead of `Dont` naming scheme.


https://github.com/llvm/llvm-project/pull/79199


More information about the llvm-commits mailing list