[RISCV] Add support for the Zawrs extension

[palmer-dabbelt]

Summary

This extension contains a single instruction, wrs, which waits for the
current reservation set to be modified by a remote agent.

Signed-off-by: Palmer Dabbelt <[e-mail]>

---

This is still some sort of very early draft and I'm not sure what the
rules are in LLVM land for merging that sort of code. It's a fast track
extension so it should be done soon, but it's been drifting around a bit
as part of that process so this might not be sane right now. I'm really
just doing this to check the box on LLVM support, which is just
assembler support due to the nature of the WRS instruction.

I also haven't gotten clang-format working yet, but I'm hoping something
in the CI will help with that.

[palmer-dabbelt]

I've never touched LLVM before, so I have no idea if I'm even in the right place here...

[eopXD]

llvm/lib/Target/RISCV/RISCVInstrInfoZawrs.td

1

This doesn't match the filename.

[eopXD]

In D128235#3597645, @palmer-dabbelt wrote:

I've never touched LLVM before, so I have no idea if I'm even in the right place here...

Yes, you are in the right place.
All LLVM patches go through the Phabricator.
A patch goes in when at least one person accepts it.

[eopXD]

• I think we will need some test coverage under llvm/test/CodeGen/RISCV/attributes.ll and llvm/test/MC/RISCV/attribute-arch.s if the extension is existing in the arch string.
• Since macro was added, we need some test coverage under clang/test/Preprocessor/riscv-target-features.c
• There are also some test cases for error handling under clang/test/Driver/riscv-arch.c, maybe add some there too?

[palmer-dabbelt]

• Fix a whitespace issue
• Fix the name of RISCVInstrInfoZawrs.td
• Call the #define riscv_zawrs instead of riscv_wrs, to match the others
• Add tests in clang/test/Driver/riscv-arch.c, clang/test/Preprocessor/riscv-target-features.c, llvm/test/CodeGen/RISCV/attributes.ll

The clang tests aren't running for me, presumably because clang isn't building locally. Also hoping to rely on CI for that one, but I'm not entirely sure.

[eopXD]

Looks like the patch application failed, might need to rebase.

[craig.topper]

clang/test/Driver/riscv-arch.c

585 ↗

(On Diff #438515)

This doesn't match the RUN line

589 ↗

(On Diff #438515)

This doesn't match the RUN line

clang/test/Preprocessor/riscv-target-features.c

444 ↗

(On Diff #438515)

This CHECK directive doesn't match the RUN line

[MaskRay]

Context not available.

See -U999999 on https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface or use arc diff (PHP 8 may need https://discourse.llvm.org/t/arcanist-deprecation-errors-on-php-8/63231)

llvm/test/MC/RISCV/zawrs-valid.s

3

The convention is to use --check-prefix is there is only one prefix.

13

Append {{$}} to make the test stricter.

[MaskRay]

llvm/test/MC/RISCV/zawrs-invalid.s

5

[[@LINE]] is a deprecated FileCheck feature.

Use [[#@LINE]]

Note: you can place negative tests with the positive test file. Search .ifndef err.

[kito-cheng]

clang/lib/Basic/Targets/RISCV.cpp

195

You don't need this line, that would be defined automatically once you've add zawrs in RISCVISAInfo.cpp, see the loop in the earlier in this function, it's done by there:

for (auto &Extension : ISAInfo->getExtensions()) {
  auto ExtName = Extension.first;
  auto ExtInfo = Extension.second;
  unsigned Version =
      (ExtInfo.MajorVersion * 1000000) + (ExtInfo.MinorVersion * 1000);

  Builder.defineMacro(Twine("__riscv_", ExtName), Twine(Version));
}

[asb]

This is still some sort of very early draft and I'm not sure what the rules are in LLVM land for merging that sort of code. It's a fast track extension so it should be done soon

The policy we ended up with is that support for not-yet-ratified extensions can be merged, but they're made to stick out like a sore thumb by calling the feature -experimental-foo and requiring -menable-experimental-extensions in clang. Given the simplicity of zawrs there's substantially less scope for drastic changes, but I think conforming to that is probably the best patch forwards. You should be able to see an example by looking at one of the stalled bitmanip extensions, e.g. experimental-zbp.

[palmer-dabbelt]

I was talking to @reames about this. The spec has changed since I posted these patches: there's now two instructions (wrs.nto and wrs.sto) and there's no longer a register to encode the timeout.

clang/lib/Basic/Targets/RISCV.cpp

if (MinVLen) {
     Builder.defineMacro("__riscv_v_elen_fp", Twine(MaxELenFp));
   }
 
   if (ISAInfo->hasExtension("c"))
     Builder.defineMacro("__riscv_compressed");
 
   if (ISAInfo->hasExtension("zve32x"))
     Builder.defineMacro("__riscv_vector");
+  
+  if (ISAInfo->hasExtension("zawrs"))
+    Builder.defineMacro("__riscv_wrs");
 }
 
 const Builtin::Info RISCVTargetInfo::BuiltinInfo[] = {
 #define BUILTIN(ID, TYPE, ATTRS)                                               \
   {#ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, nullptr},
 #define TARGET_BUILTIN(ID, TYPE, ATTRS, FEATURE)                               \
     {#ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, FEATURE},
 #include "clang/Basic/BuiltinsRISCVVector.def"

llvm/lib/Support/RISCVISAInfo.cpp

static const RISCVSupportedExtension SupportedExtensions[] = {
     {"m", RISCVExtensionVersion{2, 0}},
     {"a", RISCVExtensionVersion{2, 0}},
     {"f", RISCVExtensionVersion{2, 0}},
     {"d", RISCVExtensionVersion{2, 0}},
     {"c", RISCVExtensionVersion{2, 0}},
 
     {"zihintpause", RISCVExtensionVersion{2, 0}},
 
+    {"zawrs", RISCVExtensionVersion{1, 0}},
+
     {"zfhmin", RISCVExtensionVersion{1, 0}},
     {"zfh", RISCVExtensionVersion{1, 0}},
 
     {"zfinx", RISCVExtensionVersion{1, 0}},
     {"zdinx", RISCVExtensionVersion{1, 0}},
     {"zhinxmin", RISCVExtensionVersion{1, 0}},
     {"zhinx", RISCVExtensionVersion{1, 0}},
 

llvm/lib/Target/RISCV/RISCV.td

def HasVInstructionsAnyF : Predicate<"Subtarget->hasVInstructionsAnyF()">,
           "'V' (Vector Extension for Application Processors), 'Zve32f', "
           "'Zve64f' or 'Zve64d' (Vector Extensions for Embedded Processors)">;
 
 def FeatureStdExtZvfh
     : SubtargetFeature<"experimental-zvfh", "HasStdExtZvfh", "true",
                        "'Zvfh' (Vector Half-Precision Floating-Point)",
                        [FeatureStdExtZve32f]>;
 
+def FeatureStdExtZawrs
+    : SubtargetFeature<"zawrs", "HasStdExtZawrs", "true",
+                       "'Zawrs' (Wait Reservation Set)">;
+def HasStdExtZawrs
+    : Predicate<"Subtarget->hasStdExtZawrs">,
+                AssemblerPredicate<(any_of FeatureStdExtZawrs),
+                                   "'Zawrs' (Wait Reservation Set) ">;
+
 def Feature64Bit
     : SubtargetFeature<"64bit", "HasRV64", "true", "Implements RV64">;
 def IsRV64 : Predicate<"Subtarget->is64Bit()">,
                        AssemblerPredicate<(all_of Feature64Bit),
                        "RV64I Base Instruction Set">;
 def IsRV32 : Predicate<"!Subtarget->is64Bit()">,
                        AssemblerPredicate<(all_of (not Feature64Bit)),
                        "RV32I Base Instruction Set">;

llvm/lib/Target/RISCV/RISCVInstrInfo.td

 include "RISCVInstrInfoA.td"
 include "RISCVInstrInfoF.td"
 include "RISCVInstrInfoD.td"
 include "RISCVInstrInfoC.td"
 include "RISCVInstrInfoZb.td"
 include "RISCVInstrInfoZk.td"
 include "RISCVInstrInfoV.td"
 include "RISCVInstrInfoZfh.td"
+include "RISCVInstrInfoZawrs.td"

llvm/lib/Target/RISCV/RISCVInstrInfoZawrs.td

+//===-- RISCVInstrInfoM.td - RISC-V 'M' instructions -------*- tablegen -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file describes the RISC-V instructions from the standard 'Zawrs', Wait
+// Reservation Set, extension.
+//
+//===----------------------------------------------------------------------===//
+
+let Predicates = [HasStdExtZawrs], hasSideEffects = 1, mayStore = 0, mayLoad = 0 in {
+  def WRS : RVInstI<0b000, OPC_SYSTEM, (outs), (ins GPR:$rs1), "wrs", "$rs1">, Sched<[]> {
+    let rd = 0;
+    let imm12 = 0x010;
+  }
+}
+
+def : InstAlias<"wrs", (WRS X0)>;

llvm/lib/Target/RISCV/RISCVSubtarget.h

private:
   bool HasStdExtZknh = false;
   bool HasStdExtZksed = false;
   bool HasStdExtZksh = false;
   bool HasStdExtZkr = false;
   bool HasStdExtZkn = false;
   bool HasStdExtZks = false;
   bool HasStdExtZkt = false;
   bool HasStdExtZk = false;
+  bool HasStdExtZawrs = false;
   bool HasRV64 = false;
   bool IsRV32E = false;
   bool EnableLinkerRelax = false;
   bool EnableRVCHintInstrs = true;
   bool EnableDefaultUnroll = true;
   bool EnableSaveRestore = false;
   bool EnableUnalignedScalarMem = false;
   unsigned XLen = 32;
public:
   bool hasStdExtZbkc() const { return HasStdExtZbkc; }
   bool hasStdExtZbkx() const { return HasStdExtZbkx; }
   bool hasStdExtZknd() const { return HasStdExtZknd; }
   bool hasStdExtZkne() const { return HasStdExtZkne; }
   bool hasStdExtZknh() const { return HasStdExtZknh; }
   bool hasStdExtZksed() const { return HasStdExtZksed; }
   bool hasStdExtZksh() const { return HasStdExtZksh; }
   bool hasStdExtZkr() const { return HasStdExtZkr; }
+  bool hasStdExtZawrs() const { return HasStdExtZawrs; }
   bool is64Bit() const { return HasRV64; }
   bool isRV32E() const { return IsRV32E; }
   bool enableLinkerRelax() const { return EnableLinkerRelax; }
   bool enableRVCHintInstrs() const { return EnableRVCHintInstrs; }
   bool enableDefaultUnroll() const { return EnableDefaultUnroll; }
   bool enableSaveRestore() const { return EnableSaveRestore; }
   bool enableUnalignedScalarMem() const { return EnableUnalignedScalarMem; }
   MVT getXLenVT() const { return XLenVT; }

llvm/test/MC/RISCV/zawrs-invalid.s

+# RUN: not llvm-mc -triple riscv64 -mattr=+a < %s 2>&1 | FileCheck %s
+
+# WRS doesn't take immediates
+wrs 1 # CHECK: :[[@LINE]]:5: error: invalid operand for instruction
+
+# WRS only takes at most a single register
+wrs t0, 1 # CHECK: :[[@LINE]]:9: error: invalid operand for instruction
+wrs t0, t0 # CHECK: :[[@LINE]]:9: error: invalid operand for instruction

llvm/test/MC/RISCV/zawrs-valid.s

+# RUN: llvm-mc %s -triple=riscv32 -mattr=+zawrs \
+# RUN:     | FileCheck -check-prefixes=CHECK-S-OBJ %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+zawrs \
+# RUN:     | FileCheck -check-prefixes=CHECK-S-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+zawrs < %s \
+# RUN:     | llvm-objdump --mattr=+zawrs -d -r - \
+# RUN:     | FileCheck -check-prefixes=CHECK-S-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+zawrs < %s \
+# RUN:     | llvm-objdump --mattr=+zawrs -d -r - \
+# RUN:     | FileCheck -check-prefixes=CHECK-S-OBJ %s
+
+# CHECK-S-OBJ: wrs
+wrs
+
+# CHECK-S-OBJ: wrs t0
+wrs t0