Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RISCV] Add back SiFive's cdiscard.d.l1, cflush.d.l1, and cease instructions. #83896

Merged
merged 9 commits into from
Mar 13, 2024

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Mar 4, 2024

These were in LLVM 17 but removed from LLVM 18 due to an incorrect extension name being used.

This restores them with new extension names that match SiFive's downstream compiler. The extension name has been used internally for some time. It uses XSiFive instead of XSf like the newer extensions. cease did not have an internal extension name so its using the XSf convention.

The spec for the instructions is here https://sifive.cdn.prismic.io/sifive/767804da-53b2-4893-97d5-b7c030ae0a94_s76mc_core_complex_manual_21G3.pdf though the extension name is not listed.

Column width in the extension printing had to be changed to accommodate a longer extension name.

@topperc topperc requested review from asb, preames and apazos March 4, 2024 19:29
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V llvm:support labels Mar 4, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 4, 2024

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

@llvm/pr-subscribers-clang

Author: Craig Topper (topperc)

Changes

These were in LLVM 17 but removed from LLVM 18 due to an incorrect extension name being used.

This restores them with new extension names that match SiFive's downstream compiler. The extension name has been used internally for some time. It uses XSiFive instead of XSf like the newer extensions.

The spec for the instructions is here https://sifive.cdn.prismic.io/sifive/767804da-53b2-4893-97d5-b7c030ae0a94_s76mc_core_complex_manual_21G3.pdf though the extension name is not listed.

Column width in the extension printing had to be changed to accomodate a longer extension name.


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

7 Files Affected:

  • (modified) clang/test/Preprocessor/riscv-target-features.c (+18)
  • (modified) llvm/docs/RISCVUsage.rst (+6)
  • (modified) llvm/lib/Support/RISCVISAInfo.cpp (+3-1)
  • (modified) llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp (+6)
  • (modified) llvm/lib/Target/RISCV/RISCVFeatures.td (+16)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoXSf.td (+22)
  • (modified) llvm/unittests/Support/RISCVISAInfoTest.cpp (+163-161)
diff --git a/clang/test/Preprocessor/riscv-target-features.c b/clang/test/Preprocessor/riscv-target-features.c
index 664279cb123949..f8d4800aabbb44 100644
--- a/clang/test/Preprocessor/riscv-target-features.c
+++ b/clang/test/Preprocessor/riscv-target-features.c
@@ -60,6 +60,8 @@
 // CHECK-NOT: __riscv_xsfvfwmaccqqq {{.*$}}
 // CHECK-NOT: __riscv_xsfqmaccdod {{.*$}}
 // CHECK-NOT: __riscv_xsfvqmaccqoq {{.*$}}
+// CHECK-NOT: __riscv_sifivecdiscarddlone {{.*$}}
+// CHECK-NOT: __riscv_sifivecflushdlone {{.*$}}
 // CHECK-NOT: __riscv_xtheadba {{.*$}}
 // CHECK-NOT: __riscv_xtheadbb {{.*$}}
 // CHECK-NOT: __riscv_xtheadbs {{.*$}}
@@ -548,6 +550,22 @@
 // RUN:   -o - | FileCheck --check-prefix=CHECK-XSFVQMACCQOQ-EXT %s
 // CHECK-XSFVQMACCQOQ-EXT: __riscv_xsfvqmaccqoq 1000000{{$}}
 
+// RUN: %clang --target=riscv32-unknown-linux-gnu \
+// RUN:   -march=rv32ixsifivecdiscarddlone -E -dM %s \
+// RUN:   -o - | FileCheck --check-prefix=CHECK-XSIFIVECDISCARDDLONE-EXT %s
+// RUN: %clang --target=riscv64-unknown-linux-gnu \
+// RUN:   -march=rv64ixsifivecdiscarddlone -E -dM %s \
+// RUN:   -o - | FileCheck --check-prefix=CHECK-XSIFIVECDISCARDDLONE-EXT %s
+// CHECK-XSIFIVECDISCARDDLONE-EXT: __riscv_xsifivecdiscarddlone 1000000{{$}}
+
+// RUN: %clang --target=riscv32-unknown-linux-gnu \
+// RUN:   -march=rv32ixsifivecflushdlone -E -dM %s \
+// RUN:   -o - | FileCheck --check-prefix=CHECK-XSIFIVECFLUSHDLONE-EXT %s
+// RUN: %clang --target=riscv64-unknown-linux-gnu \
+// RUN:   -march=rv64ixsifivecflushdlone -E -dM %s \
+// RUN:   -o - | FileCheck --check-prefix=CHECK-XSIFIVECFLUSHDLONE-EXT %s
+// CHECK-XSIFIVECFLUSHDLONE-EXT: __riscv_xsifivecflushdlone 1000000{{$}}
+
 // RUN: %clang --target=riscv32-unknown-linux-gnu \
 // RUN:   -march=rv32ixtheadba -E -dM %s \
 // RUN:   -o - | FileCheck --check-prefix=CHECK-XTHEADBA-EXT %s
diff --git a/llvm/docs/RISCVUsage.rst b/llvm/docs/RISCVUsage.rst
index 8d293b02144307..f1e2e86390c838 100644
--- a/llvm/docs/RISCVUsage.rst
+++ b/llvm/docs/RISCVUsage.rst
@@ -352,6 +352,12 @@ The current vendor extensions supported are:
 ``XCVbi``
   LLVM implements `version 1.0.0 of the CORE-V immediate branching custom instructions specification <https://github.com/openhwgroup/cv32e40p/blob/cv32e40p_v1.3.2/docs/source/instruction_set_extensions.rst>`__ by OpenHW Group.  All instructions are prefixed with `cv.` as described in the specification. These instructions are only available for riscv32 at this time.
 
+``XSiFivecdiscarddlone``
+  LLVM implements `the SiFive cdiscard.d.l1 instruction specified in <https://sifive.cdn.prismic.io/sifive/767804da-53b2-4893-97d5-b7c030ae0a94_s76mc_core_complex_manual_21G3.pdf>`_ by SiFive.
+
+``XSiFivecflushdlone``
+  LLVM implements `the SiFive cflush.d.l1 instruction specified in <https://sifive.cdn.prismic.io/sifive/767804da-53b2-4893-97d5-b7c030ae0a94_s76mc_core_complex_manual_21G3.pdf>`_ by SiFive.
+
 Experimental C Intrinsics
 =========================
 
diff --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp
index 68f5c36e8fafc6..8dbb40f97bf338 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -94,6 +94,8 @@ static const RISCVSupportedExtension SupportedExtensions[] = {
     {"xsfvfwmaccqqq", {1, 0}},
     {"xsfvqmaccdod", {1, 0}},
     {"xsfvqmaccqoq", {1, 0}},
+    {"xsifivecdiscarddlone", {1, 0}},
+    {"xsifivecflushdlone", {1, 0}},
     {"xtheadba", {1, 0}},
     {"xtheadbb", {1, 0}},
     {"xtheadbs", {1, 0}},
@@ -257,7 +259,7 @@ static void PrintExtension(StringRef Name, StringRef Version,
                            StringRef Description) {
   outs().indent(4);
   unsigned VersionWidth = Description.empty() ? 0 : 10;
-  outs() << left_justify(Name, 20) << left_justify(Version, VersionWidth)
+  outs() << left_justify(Name, 21) << left_justify(Version, VersionWidth)
          << Description << "\n";
 }
 
diff --git a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
index f1ca1212ec378e..00518653cc494e 100644
--- a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
+++ b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
@@ -595,6 +595,12 @@ DecodeStatus RISCVDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
     TRY_TO_DECODE_FEATURE(
         RISCV::FeatureVendorXSfvfnrclipxfqf, DecoderTableXSfvfnrclipxfqf32,
         "SiFive FP32-to-int8 Ranged Clip Instructions opcode table");
+    TRY_TO_DECODE_FEATURE(RISCV::FeatureVendorXSiFivecdiscarddlone,
+                          DecoderTableXSiFivecdiscarddlone32,
+                          "SiFive cdiscard.d.l1 custom opcode table");
+    TRY_TO_DECODE_FEATURE(RISCV::FeatureVendorXSiFivecflushdlone,
+                          DecoderTableXSiFivecflushdlone32,
+                          "SiFive cflush.d.l1 custom opcode table");
     TRY_TO_DECODE_FEATURE(RISCV::FeatureVendorXCVbitmanip,
                           DecoderTableXCVbitmanip32,
                           "CORE-V Bit Manipulation custom opcode table");
diff --git a/llvm/lib/Target/RISCV/RISCVFeatures.td b/llvm/lib/Target/RISCV/RISCVFeatures.td
index 9773b2998c7dc4..fb47e5f76c5e2e 100644
--- a/llvm/lib/Target/RISCV/RISCVFeatures.td
+++ b/llvm/lib/Target/RISCV/RISCVFeatures.td
@@ -1054,6 +1054,22 @@ def HasVendorXSfvfnrclipxfqf
       AssemblerPredicate<(all_of FeatureVendorXSfvfnrclipxfqf),
                          "'XSfvfnrclipxfqf' (SiFive FP32-to-int8 Ranged Clip Instructions)">;
 
+def FeatureVendorXSiFivecdiscarddlone
+    : SubtargetFeature<"xsifivecdiscarddlone", "HasVendorXSiFivecdiscarddlone", "true",
+                       "'XSiFivecdiscarddlone' (SiFive cdiscard.d.l1 Instruction)", []>;
+def HasVendorXSiFivecdiscarddlone
+    : Predicate<"Subtarget->hasVendorXSiFivecdiscarddlone()">,
+      AssemblerPredicate<(all_of FeatureVendorXSiFivecdiscarddlone),
+                         "'XSiFivecdiscarddlone' (SiFive cdiscard.d.l1 Instruction)">;
+
+def FeatureVendorXSiFivecflushdlone
+    : SubtargetFeature<"xsifivecflushdlone", "HasVendorXSiFivecflushdlone", "true",
+                       "'XSiFivecflushdlone' (SiFive cflush.d.l1 Instruction)", []>;
+def HasVendorXSiFivecflushdlone
+    : Predicate<"Subtarget->hasVendorXSiFivecflushdlone()">,
+      AssemblerPredicate<(all_of FeatureVendorXSiFivecflushdlone),
+                         "'XSiFivecflushdlone' (SiFive cflush.d.l1 Instruction)">;
+
 // Core-V Extensions
 
 def FeatureVendorXCVelw
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXSf.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXSf.td
index b4130e3805a110..a7601724300b64 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoXSf.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXSf.td
@@ -808,3 +808,25 @@ let Predicates = [HasVendorXSfvfnrclipxfqf] in {
   defm : VPatVFNRCLIP<"vfnrclip_xu_f_qf", "VFNRCLIP_XU_F_QF">;
   defm : VPatVFNRCLIP<"vfnrclip_x_f_qf", "VFNRCLIP_X_F_QF">;
 }
+
+let Predicates = [HasVendorXSiFivecdiscarddlone] in {
+  let hasNoSchedulingInfo = 1, hasSideEffects = 1, mayLoad = 0, mayStore = 0,
+      DecoderNamespace = "XSiFivecdiscarddlone" in
+  def SF_CDISCARD_D_L1
+      : RVInstR<0b1111110, 0b000, OPC_SYSTEM, (outs), (ins GPR:$rs1),
+                "cdiscard.d.l1", "$rs1">, Sched<[]> {
+    let rd = 0;
+    let rs2 = 0b00010;
+  }
+} // Predicates = [HasVendorXSifivecdiscarddlone]
+
+let Predicates = [HasVendorXSiFivecflushdlone] in {
+  let hasNoSchedulingInfo = 1, hasSideEffects = 1, mayLoad = 0, mayStore = 0,
+      DecoderNamespace = "XSiFivecflushdlone" in
+  def SF_CFLUSH_D_L1
+      : RVInstR<0b1111110, 0b000, OPC_SYSTEM, (outs), (ins GPR:$rs1),
+                "cflush.d.l1", "$rs1">, Sched<[]> {
+    let rd = 0;
+    let rs2 = 0b00000;
+  }
+} // Predicates = [HasVendorXSifivecflushdlone]
diff --git a/llvm/unittests/Support/RISCVISAInfoTest.cpp b/llvm/unittests/Support/RISCVISAInfoTest.cpp
index 82cce23638d5f5..95560f34420146 100644
--- a/llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -724,169 +724,171 @@ TEST(RiscvExtensionsHelp, CheckExtensions) {
   std::string ExpectedOutput =
 R"(All available -march extensions for RISC-V
 
-    Name                Version   Description
-    i                   2.1       This is a long dummy description
-    e                   2.0
-    m                   2.0
-    a                   2.1
-    f                   2.2
-    d                   2.2
-    c                   2.0
-    v                   1.0
-    h                   1.0
-    zic64b              1.0
-    zicbom              1.0
-    zicbop              1.0
-    zicboz              1.0
-    ziccamoa            1.0
-    ziccif              1.0
-    zicclsm             1.0
-    ziccrse             1.0
-    zicntr              2.0
-    zicond              1.0
-    zicsr               2.0
-    zifencei            2.0
-    zihintntl           1.0
-    zihintpause         2.0
-    zihpm               2.0
-    zmmul               1.0
-    za128rs             1.0
-    za64rs              1.0
-    zacas               1.0
-    zawrs               1.0
-    zfa                 1.0
-    zfh                 1.0
-    zfhmin              1.0
-    zfinx               1.0
-    zdinx               1.0
-    zca                 1.0
-    zcb                 1.0
-    zcd                 1.0
-    zce                 1.0
-    zcf                 1.0
-    zcmp                1.0
-    zcmt                1.0
-    zba                 1.0
-    zbb                 1.0
-    zbc                 1.0
-    zbkb                1.0
-    zbkc                1.0
-    zbkx                1.0
-    zbs                 1.0
-    zk                  1.0
-    zkn                 1.0
-    zknd                1.0
-    zkne                1.0
-    zknh                1.0
-    zkr                 1.0
-    zks                 1.0
-    zksed               1.0
-    zksh                1.0
-    zkt                 1.0
-    zvbb                1.0
-    zvbc                1.0
-    zve32f              1.0
-    zve32x              1.0
-    zve64d              1.0
-    zve64f              1.0
-    zve64x              1.0
-    zvfh                1.0
-    zvfhmin             1.0
-    zvkb                1.0
-    zvkg                1.0
-    zvkn                1.0
-    zvknc               1.0
-    zvkned              1.0
-    zvkng               1.0
-    zvknha              1.0
-    zvknhb              1.0
-    zvks                1.0
-    zvksc               1.0
-    zvksed              1.0
-    zvksg               1.0
-    zvksh               1.0
-    zvkt                1.0
-    zvl1024b            1.0
-    zvl128b             1.0
-    zvl16384b           1.0
-    zvl2048b            1.0
-    zvl256b             1.0
-    zvl32768b           1.0
-    zvl32b              1.0
-    zvl4096b            1.0
-    zvl512b             1.0
-    zvl64b              1.0
-    zvl65536b           1.0
-    zvl8192b            1.0
-    zhinx               1.0
-    zhinxmin            1.0
-    shcounterenw        1.0
-    shgatpa             1.0
-    shtvala             1.0
-    shvsatpa            1.0
-    shvstvala           1.0
-    shvstvecd           1.0
-    smaia               1.0
-    smepmp              1.0
-    ssaia               1.0
-    ssccptr             1.0
-    sscounterenw        1.0
-    ssstateen           1.0
-    ssstrict            1.0
-    sstc                1.0
-    sstvala             1.0
-    sstvecd             1.0
-    ssu64xl             1.0
-    svade               1.0
-    svadu               1.0
-    svbare              1.0
-    svinval             1.0
-    svnapot             1.0
-    svpbmt              1.0
-    xcvalu              1.0
-    xcvbi               1.0
-    xcvbitmanip         1.0
-    xcvelw              1.0
-    xcvmac              1.0
-    xcvmem              1.0
-    xcvsimd             1.0
-    xsfvcp              1.0
-    xsfvfnrclipxfqf     1.0
-    xsfvfwmaccqqq       1.0
-    xsfvqmaccdod        1.0
-    xsfvqmaccqoq        1.0
-    xtheadba            1.0
-    xtheadbb            1.0
-    xtheadbs            1.0
-    xtheadcmo           1.0
-    xtheadcondmov       1.0
-    xtheadfmemidx       1.0
-    xtheadmac           1.0
-    xtheadmemidx        1.0
-    xtheadmempair       1.0
-    xtheadsync          1.0
-    xtheadvdot          1.0
-    xventanacondops     1.0
+    Name                 Version   Description
+    i                    2.1       This is a long dummy description
+    e                    2.0
+    m                    2.0
+    a                    2.1
+    f                    2.2
+    d                    2.2
+    c                    2.0
+    v                    1.0
+    h                    1.0
+    zic64b               1.0
+    zicbom               1.0
+    zicbop               1.0
+    zicboz               1.0
+    ziccamoa             1.0
+    ziccif               1.0
+    zicclsm              1.0
+    ziccrse              1.0
+    zicntr               2.0
+    zicond               1.0
+    zicsr                2.0
+    zifencei             2.0
+    zihintntl            1.0
+    zihintpause          2.0
+    zihpm                2.0
+    zmmul                1.0
+    za128rs              1.0
+    za64rs               1.0
+    zacas                1.0
+    zawrs                1.0
+    zfa                  1.0
+    zfh                  1.0
+    zfhmin               1.0
+    zfinx                1.0
+    zdinx                1.0
+    zca                  1.0
+    zcb                  1.0
+    zcd                  1.0
+    zce                  1.0
+    zcf                  1.0
+    zcmp                 1.0
+    zcmt                 1.0
+    zba                  1.0
+    zbb                  1.0
+    zbc                  1.0
+    zbkb                 1.0
+    zbkc                 1.0
+    zbkx                 1.0
+    zbs                  1.0
+    zk                   1.0
+    zkn                  1.0
+    zknd                 1.0
+    zkne                 1.0
+    zknh                 1.0
+    zkr                  1.0
+    zks                  1.0
+    zksed                1.0
+    zksh                 1.0
+    zkt                  1.0
+    zvbb                 1.0
+    zvbc                 1.0
+    zve32f               1.0
+    zve32x               1.0
+    zve64d               1.0
+    zve64f               1.0
+    zve64x               1.0
+    zvfh                 1.0
+    zvfhmin              1.0
+    zvkb                 1.0
+    zvkg                 1.0
+    zvkn                 1.0
+    zvknc                1.0
+    zvkned               1.0
+    zvkng                1.0
+    zvknha               1.0
+    zvknhb               1.0
+    zvks                 1.0
+    zvksc                1.0
+    zvksed               1.0
+    zvksg                1.0
+    zvksh                1.0
+    zvkt                 1.0
+    zvl1024b             1.0
+    zvl128b              1.0
+    zvl16384b            1.0
+    zvl2048b             1.0
+    zvl256b              1.0
+    zvl32768b            1.0
+    zvl32b               1.0
+    zvl4096b             1.0
+    zvl512b              1.0
+    zvl64b               1.0
+    zvl65536b            1.0
+    zvl8192b             1.0
+    zhinx                1.0
+    zhinxmin             1.0
+    shcounterenw         1.0
+    shgatpa              1.0
+    shtvala              1.0
+    shvsatpa             1.0
+    shvstvala            1.0
+    shvstvecd            1.0
+    smaia                1.0
+    smepmp               1.0
+    ssaia                1.0
+    ssccptr              1.0
+    sscounterenw         1.0
+    ssstateen            1.0
+    ssstrict             1.0
+    sstc                 1.0
+    sstvala              1.0
+    sstvecd              1.0
+    ssu64xl              1.0
+    svade                1.0
+    svadu                1.0
+    svbare               1.0
+    svinval              1.0
+    svnapot              1.0
+    svpbmt               1.0
+    xcvalu               1.0
+    xcvbi                1.0
+    xcvbitmanip          1.0
+    xcvelw               1.0
+    xcvmac               1.0
+    xcvmem               1.0
+    xcvsimd              1.0
+    xsfvcp               1.0
+    xsfvfnrclipxfqf      1.0
+    xsfvfwmaccqqq        1.0
+    xsfvqmaccdod         1.0
+    xsfvqmaccqoq         1.0
+    xsifivecdiscarddlone 1.0
+    xsifivecflushdlone   1.0
+    xtheadba             1.0
+    xtheadbb             1.0
+    xtheadbs             1.0
+    xtheadcmo            1.0
+    xtheadcondmov        1.0
+    xtheadfmemidx        1.0
+    xtheadmac            1.0
+    xtheadmemidx         1.0
+    xtheadmempair        1.0
+    xtheadsync           1.0
+    xtheadvdot           1.0
+    xventanacondops      1.0
 
 Experimental extensions
-    zicfilp             0.4       This is a long dummy description
-    zicfiss             0.4
-    zimop               0.1
-    zaamo               0.2
-    zabha               1.0
-    zalasr              0.1
-    zalrsc              0.2
-    zfbfmin             1.0
-    zcmop               0.2
-    ztso                0.1
-    zvfbfmin            1.0
-    zvfbfwma            1.0
-    smmpm               0.8
-    smnpm               0.8
-    ssnpm               0.8
-    sspm                0.8
-    ssqosid             1.0
-    supm                0.8
+    zicfilp              0.4       This is a long dummy description
+    zicfiss              0.4
+    zimop                0.1
+    zaamo                0.2
+    zabha                1.0
+    zalasr               0.1
+    zalrsc               0.2
+    zfbfmin              1.0
+    zcmop                0.2
+    ztso                 0.1
+    zvfbfmin             1.0
+    zvfbfwma             1.0
+    smmpm                0.8
+    smnpm                0.8
+    ssnpm                0.8
+    sspm                 0.8
+    ssqosid              1.0
+    supm                 0.8
 
 Use -march to specify the target's extension.
 For example, clang -march=rv32i_v1p0)";

@llvmbot llvmbot added the mc Machine (object) code label Mar 4, 2024
jrtc27
jrtc27 previously requested changes Mar 4, 2024
Copy link
Collaborator

@jrtc27 jrtc27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These need the vendor "sf." prefix

@sunshaoce
Copy link
Contributor

By the way, is there any plan to support CFLUSH.I.L1 in the future?

@jrtc27
Copy link
Collaborator

jrtc27 commented Mar 6, 2024

By the way, is there any plan to support CFLUSH.I.L1 in the future?

Flushing the instruction cache doesn't make sense given it can never be dirty. Invalidating/discarding does, but that's just what fence.i is doing?

@topperc
Copy link
Collaborator Author

topperc commented Mar 6, 2024

By the way, is there any plan to support CFLUSH.I.L1 in the future?

Flushing the instruction cache doesn't make sense given it can never be dirty. Invalidating/discarding does, but that's just what fence.i is doing?

A cflush.i.l1 did appear in some SiFive manual at some point, but I don't think any hardware implements it.

@sunshaoce
Copy link
Contributor

By the way, is there any plan to support CFLUSH.I.L1 in the future?

Flushing the instruction cache doesn't make sense given it can never be dirty. Invalidating/discarding does, but that's just what fence.i is doing?

A cflush.i.l1 did appear in some SiFive manual at some point, but I don't think any hardware implements it.

Understood, thank you for the reply!

@topperc
Copy link
Collaborator Author

topperc commented Mar 8, 2024

@jrtc27 does this look better now?

@jrtc27 jrtc27 self-requested a review March 8, 2024 19:10
@jrtc27
Copy link
Collaborator

jrtc27 commented Mar 8, 2024

@jrtc27 does this look better now?

Yes; thanks. I believe I’ve rescinded my review.

@jrtc27 jrtc27 removed their request for review March 8, 2024 19:11
@jrtc27 jrtc27 dismissed their stale review March 8, 2024 19:11

Addressed

@apazos
Copy link
Contributor

apazos commented Mar 11, 2024

@topperc , how about the CEASE instruction? Can you add it in this patch?

@apazos
Copy link
Contributor

apazos commented Mar 11, 2024

Also, @topperc, we should add the new extensions to the list of vendor extensions in the specs:
ttps://github.com/riscv-non-isa/riscv-toolchain-conventions/tree/master?tab=readme-ov-file#list-of-vendor-extensions

@quic-garvgupt
Copy link
Contributor

Hi @topperc, can you add instruction alias for cflush and cdiscard instructions when the rs1 is X0 to sf.cflush.d.l1 and sf.cflush.d.l1 respectively, as this register is optional according to spec?

@quic-garvgupt
Copy link
Contributor

Also, I think we might need to update the extensions in the RISCVProcessors.td file under SIFIVE_S76 microcontroller?

@topperc
Copy link
Collaborator Author

topperc commented Mar 11, 2024

Also, I think we might need to update the extensions in the RISCVProcessors.td file under SIFIVE_S76 microcontroller?

This is a M-mode only extension, and we haven't historically been adding M or S mode extensions to the -mcpu lists. Except for xiangshan-nanhu having Svinval. But maybe there's a microcontroller vs application core distinction we should be making?

@quic-garvgupt
Copy link
Contributor

Hi @topperc, can you add instruction alias for cflush and cdiscard instructions when the rs1 is X0 to sf.cflush.d.l1 and sf.cflush.d.l1 respectively, as this register is optional according to spec?

x0 has special meaning, but the spec never says it is "optional".

Possibly I am mistaken here but this is what I inferred from the below text in the mannual - rs1 is optional, so if a user does not specify it then by default it will be X0. Apologies if I am missing something here but just wanted to be clear.

From the mannual -
Opcode 0xFC000073, with **optional** rs1 field in bits [19:15]

@quic-garvgupt
Copy link
Contributor

Also, I think we might need to update the extensions in the RISCVProcessors.td file under SIFIVE_S76 microcontroller?

This is a M-mode only extension, and we haven't historically been adding M or S mode extensions to the -mcpu lists. Except for xiangshan-nanhu having Svinval. But maybe there's a microcontroller vs application core distinction we should be making?

I see, I was not aware of this. Thanks for your reply

@topperc
Copy link
Collaborator Author

topperc commented Mar 12, 2024

Hi @topperc, can you add instruction alias for cflush and cdiscard instructions when the rs1 is X0 to sf.cflush.d.l1 and sf.cflush.d.l1 respectively, as this register is optional according to spec?

x0 has special meaning, but the spec never says it is "optional".

Possibly I am mistaken here but this is what I inferred from the below text in the mannual - rs1 is optional, so if a user does not specify it then by default it will be X0. Apologies if I am missing something here but just wanted to be clear.

From the mannual - Opcode 0xFC000073, with **optional** rs1 field in bits [19:15]

I think i had a bad copy of the file. There are several iterations. And one of the copies I had doesn't have the "optional" text on cflush.d.l1.

These were in LLVM 17 but removed from LLVM 18 due to an incorrect
extension name being used.

This restores them with new extension names that match SiFive's
downstream compiler. The extension name has been used internally
for some time. It uses XSiFive instead of XSf like the newer extensions.

The spec for the instructions is here https://sifive.cdn.prismic.io/sifive/767804da-53b2-4893-97d5-b7c030ae0a94_s76mc_core_complex_manual_21G3.pdf
though the extension name is not listed.

Column width in the extension printing had to be changed to accomodate
a longer extension name.
Copy link

github-actions bot commented Mar 12, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@quic-garvgupt
Copy link
Contributor

quic-garvgupt commented Mar 13, 2024

Thanks for clearing the confusion around whether rs1 would be optional or not.

@topperc
Copy link
Collaborator Author

topperc commented Mar 13, 2024

Thanks for clearing the confusion around whether rs1 would be optional or not. Can we also add lit tests for the aliases?

I already addded tests f6f43e9

@quic-garvgupt
Copy link
Contributor

Thanks for the prompt reply and latest patchset.

  1. Do we need to add documentation in RISCVUsage.rst file for xsfcease?
  2. Also, as we are adding cease instruction in this PR, can we rename the PR to include the cease instruction as well?

@topperc topperc changed the title [RISCV] Add back SiFive's cdiscard.d.l1 and cflush.d.l1 instructions. [RISCV] Add back SiFive's cdiscard.d.l1, cflush.d.l1, and cease instructions. Mar 13, 2024
@quic-garvgupt
Copy link
Contributor

I believe the patch is ready to be merged now, but since I am not the reviewer here, someone else must approve it as well.

Thanks again for the patch!

@topperc topperc requested review from dtcxzyw and jrtc27 March 13, 2024 16:53
@topperc topperc merged commit 207e45f into llvm:main Mar 13, 2024
5 checks passed
@topperc topperc deleted the pr/xsifive-instructions branch March 13, 2024 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V clang Clang issues not falling into any other category llvm:support mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants