Skip to content

Conversation

@wangpc-pp
Copy link
Contributor

This is a compromise of #114518.

We may also add a new extension Zvnotrapvmvnr or whatever that
doesn't add new instructions but these instructions won't trap on
vill to fix this mistake.

Not all of us want to pay for the mistake.

…on vill

This is a compromise of llvm#114518.

We may also add a new extension `Zvnotrapvmvnr` or whatever that
doesn't add new instructions but these instructions won't trap on
vill to fix this mistake.

Not all of us want to pay for the mistake.
@llvmbot
Copy link
Member

llvmbot commented Nov 5, 2024

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

Author: Pengcheng Wang (wangpc-pp)

Changes

This is a compromise of #114518.

We may also add a new extension Zvnotrapvmvnr or whatever that
doesn't add new instructions but these instructions won't trap on
vill to fix this mistake.

Not all of us want to pay for the mistake.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVFeatures.td (+19)
  • (modified) llvm/lib/Target/RISCV/RISCVProcessors.td (+2-1)
diff --git a/llvm/lib/Target/RISCV/RISCVFeatures.td b/llvm/lib/Target/RISCV/RISCVFeatures.td
index f2e661f007d115..6ee680c6a4956b 100644
--- a/llvm/lib/Target/RISCV/RISCVFeatures.td
+++ b/llvm/lib/Target/RISCV/RISCVFeatures.td
@@ -1381,6 +1381,25 @@ def FeaturePredictableSelectIsExpensive
     : SubtargetFeature<"predictable-select-expensive", "PredictableSelectIsExpensive", "true",
                        "Prefer likely predicted branches over selects">;
 
+// This is not a feature that is documented in the RVV spec, but a result of
+// messy change history of the whole register move.
+//
+// The whole register move should be designed as ignoring the vtype, but it was
+// somehow constrained to specific microarchitecture and got a dependency of
+// vtype. Because people didn't notice the impact of this mistake and the spec
+// is very vague and self-contradictory in several places, many cores that are
+// already on the market didn't implement the whole register move in the trapped
+// way.
+//
+// This feature is used to indicate that the implementation won't trap on vill
+// so that no extra vset(i)vl(i) is needed before the whole register move
+// instructions.
+// 
+// See #114518 for more details.
+def FeatureNoTrappedWholeRegisterMove
+    : SubtargetFeature<"no-trapped-whole-register-move", "HasTrappedWholeRegisterMove", "false",
+                       "The whole register move won't trap on vill">;
+
 def TuneOptimizedZeroStrideLoad
    : SubtargetFeature<"optimized-zero-stride-load", "HasOptimizedZeroStrideLoad",
                       "true", "Optimized (perform fewer memory operations)"
diff --git a/llvm/lib/Target/RISCV/RISCVProcessors.td b/llvm/lib/Target/RISCV/RISCVProcessors.td
index 5277752a38ad9e..0eaa1fe9d7819a 100644
--- a/llvm/lib/Target/RISCV/RISCVProcessors.td
+++ b/llvm/lib/Target/RISCV/RISCVProcessors.td
@@ -476,7 +476,8 @@ def SPACEMIT_X60 : RISCVProcessorModel<"spacemit-x60",
                                         FeatureStdExtZicond,
                                         FeatureStdExtZvfh,
                                         FeatureStdExtZvkt,
-                                        FeatureStdExtZvl256b]),
+                                        FeatureStdExtZvl256b,
+                                        FeatureNoTrappedWholeRegisterMove]),
                                        [TuneDLenFactor2,
                                         TuneOptimizedNF2SegmentLoadStore,
                                         TuneOptimizedNF3SegmentLoadStore,

@preames
Copy link
Collaborator

preames commented Nov 8, 2024

This got discussed at the RISCV sync up yesterday, but I wanted to record a quick summary. We would like to not see a forking in code generation based on this property. There is a desire to see if we can fix this through either a psabi change or a compiler change in a manner which doesn't impose a performance hit. If we can, a change like this is undesirable. If we can't, we will revisit.

@wangpc-pp wangpc-pp closed this Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants