[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

In D133443#3801899 , @asb wrote:

> It looks like they're still missing in this updated version of the patch?

I have no idea what's going wrong here.  I had been very careful to make sure 
the patch contained the new test file, but you're right, the revision in phab 
didn't.

Since we now had two LGTMs, and the tests had been in the original patch 
upload, I went ahead and landed.  The tests are in the committed patch; if you 
want any changes, let me know.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133443/new/

https://reviews.llvm.org/D133443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Philip Reames via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGeda2af575fdf: [RISCV][MC] Add support for experimental Zawrs 
extension (authored by reames).

Changed prior to commit:
  https://reviews.llvm.org/D133443?vs=461345=461608#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133443/new/

https://reviews.llvm.org/D133443

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/docs/RISCVUsage.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfo.td
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/Zawrs-valid.s
  llvm/test/MC/RISCV/attribute-arch.s

Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/attribute-arch.s
@@ -197,5 +197,8 @@
 .attribute arch, "rv32izca0p70"
 # CHECK: attribute  5, "rv32i2p0_zca0p70"
 
+.attribute arch, "rv32izawrs1p0"
+# CHECK: attribute  5, "rv32i2p0_zawrs1p0"
+
 .attribute arch, "rv32iztso0p1"
 # CHECK: attribute  5, "rv32i2p0_ztso0p1"
Index: llvm/test/MC/RISCV/Zawrs-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/Zawrs-valid.s
@@ -0,0 +1,18 @@
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-zawrs -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+experimental-zawrs -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+experimental-zawrs < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zawrs -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+experimental-zawrs < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zawrs -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
+
+# CHECK-ASM-AND-OBJ: wrs.nto
+# CHECK-ASM: encoding: [0x73,0x00,0xd0,0x00]
+wrs.nto
+
+# CHECK-ASM-AND-OBJ: wrs.sto
+# CHECK-ASM: encoding: [0x73,0x00,0xd0,0x01]
+wrs.sto
Index: llvm/test/CodeGen/RISCV/attributes.ll
===
--- llvm/test/CodeGen/RISCV/attributes.ll
+++ llvm/test/CodeGen/RISCV/attributes.ll
@@ -84,6 +84,7 @@
 ; RUN: llc -mtriple=riscv64 -mattr=+zicbom %s -o - | FileCheck --check-prefix=RV64ZICBOM %s
 ; RUN: llc -mtriple=riscv64 -mattr=+zicboz %s -o - | FileCheck --check-prefix=RV64ZICBOZ %s
 ; RUN: llc -mtriple=riscv64 -mattr=+zicbop %s -o - | FileCheck --check-prefix=RV64ZICBOP %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zawrs %s -o - | FileCheck --check-prefix=RV64ZAWRS %s
 ; RUN: llc -mtriple=riscv64 -mattr=+experimental-ztso %s -o - | FileCheck --check-prefix=RV64ZTSO %s
 
 ; RV32M: .attribute 5, "rv32i2p0_m2p0"
@@ -170,6 +171,7 @@
 ; RV64COMBINEINTOZKS: .attribute 5, "rv64i2p0_zbkb1p0_zbkc1p0_zbkx1p0_zks1p0_zksed1p0_zksh1p0"
 ; RV64ZICBOM: .attribute 5, "rv64i2p0_zicbom1p0"
 ; RV64ZICBOZ: .attribute 5, "rv64i2p0_zicboz1p0"
+; RV64ZAWRS: .attribute 5, "rv64i2p0_zawrs1p0"
 ; RV64ZICBOP: .attribute 5, "rv64i2p0_zicbop1p0"
 ; RV64ZTSO: .attribute 5, "rv64i2p0_ztso0p1"
 
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -92,6 +92,7 @@
   bool HasStdExtZicboz = false;
   bool HasStdExtZicbop = false;
   bool HasStdExtZmmul = false;
+  bool HasStdExtZawrs = false;
   bool HasStdExtZtso = false;
   bool HasRV32 = false;
   bool HasRV64 = false;
@@ -192,6 +193,7 @@
   bool hasStdExtZicbom() const { return HasStdExtZicbom; }
   bool hasStdExtZicboz() const { return HasStdExtZicboz; }
   bool hasStdExtZicbop() const { return HasStdExtZicbop; }
+  bool hasStdExtZawrs() const { return HasStdExtZawrs; }
   bool hasStdExtZmmul() const { return HasStdExtZmmul; }
   bool hasStdExtZtso() const { return HasStdExtZtso; }
   bool is64Bit() const { return HasRV64; }
Index: llvm/lib/Target/RISCV/RISCVInstrInfo.td
===
--- llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -705,6 +705,23 @@
   let rd = 0;
   let imm12 = 0b1100;
 }
+
+let Predicates = [HasStdExtZawrs] in {
+def WRS_NTO : RVInstI<0b000, OPC_SYSTEM, (outs), (ins), "wrs.nto", "">,
+  Sched<[]> {
+  let rs1 = 0;
+  let rd = 0;
+  let imm12 = 0b1101;
+}
+
+def WRS_STO : RVInstI<0b000, OPC_SYSTEM, (outs), (ins), "wrs.sto", "">,
+  Sched<[]> {
+  let rs1 = 0;
+  let rd = 0;
+  let imm12 

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Luís Marques via Phabricator via cfe-commits]

luismarques added a comment.

Regarding the overkill of "RISCVInstrInfoZawrs.td", how about having a 
"RISCVInstrInfoExtra.td" (or "RISCVInstrInfoExt.td") as a grab bag for 
everything that doesn't merit its own .td file?

I think this is fine regarding the versioning issue. Seems to only be missing 
the actual instruction tests, otherwise LGTM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133443/new/

https://reviews.llvm.org/D133443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

It looks like they're still missing in this updated version of the patch?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133443/new/

https://reviews.llvm.org/D133443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Philip Reames via Phabricator via cfe-commits]

reames updated this revision to Diff 461345.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133443/new/

https://reviews.llvm.org/D133443

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/docs/RISCVUsage.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfo.td
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s

Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/attribute-arch.s
@@ -197,5 +197,8 @@
 .attribute arch, "rv32izca0p70"
 # CHECK: attribute  5, "rv32i2p0_zca0p70"
 
+.attribute arch, "rv32izawrs1p0"
+# CHECK: attribute  5, "rv32i2p0_zawrs1p0"
+
 .attribute arch, "rv32iztso0p1"
 # CHECK: attribute  5, "rv32i2p0_ztso0p1"
Index: llvm/test/CodeGen/RISCV/attributes.ll
===
--- llvm/test/CodeGen/RISCV/attributes.ll
+++ llvm/test/CodeGen/RISCV/attributes.ll
@@ -84,6 +84,7 @@
 ; RUN: llc -mtriple=riscv64 -mattr=+zicbom %s -o - | FileCheck --check-prefix=RV64ZICBOM %s
 ; RUN: llc -mtriple=riscv64 -mattr=+zicboz %s -o - | FileCheck --check-prefix=RV64ZICBOZ %s
 ; RUN: llc -mtriple=riscv64 -mattr=+zicbop %s -o - | FileCheck --check-prefix=RV64ZICBOP %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zawrs %s -o - | FileCheck --check-prefix=RV64ZAWRS %s
 ; RUN: llc -mtriple=riscv64 -mattr=+experimental-ztso %s -o - | FileCheck --check-prefix=RV64ZTSO %s
 
 ; RV32M: .attribute 5, "rv32i2p0_m2p0"
@@ -170,6 +171,7 @@
 ; RV64COMBINEINTOZKS: .attribute 5, "rv64i2p0_zbkb1p0_zbkc1p0_zbkx1p0_zks1p0_zksed1p0_zksh1p0"
 ; RV64ZICBOM: .attribute 5, "rv64i2p0_zicbom1p0"
 ; RV64ZICBOZ: .attribute 5, "rv64i2p0_zicboz1p0"
+; RV64ZAWRS: .attribute 5, "rv64i2p0_zawrs1p0"
 ; RV64ZICBOP: .attribute 5, "rv64i2p0_zicbop1p0"
 ; RV64ZTSO: .attribute 5, "rv64i2p0_ztso0p1"
 
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -92,6 +92,7 @@
   bool HasStdExtZicboz = false;
   bool HasStdExtZicbop = false;
   bool HasStdExtZmmul = false;
+  bool HasStdExtZawrs = false;
   bool HasStdExtZtso = false;
   bool HasRV32 = false;
   bool HasRV64 = false;
@@ -192,6 +193,7 @@
   bool hasStdExtZicbom() const { return HasStdExtZicbom; }
   bool hasStdExtZicboz() const { return HasStdExtZicboz; }
   bool hasStdExtZicbop() const { return HasStdExtZicbop; }
+  bool hasStdExtZawrs() const { return HasStdExtZawrs; }
   bool hasStdExtZmmul() const { return HasStdExtZmmul; }
   bool hasStdExtZtso() const { return HasStdExtZtso; }
   bool is64Bit() const { return HasRV64; }
Index: llvm/lib/Target/RISCV/RISCVInstrInfo.td
===
--- llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -705,6 +705,23 @@
   let rd = 0;
   let imm12 = 0b1100;
 }
+
+let Predicates = [HasStdExtZawrs] in {
+def WRS_NTO : RVInstI<0b000, OPC_SYSTEM, (outs), (ins), "wrs.nto", "">,
+  Sched<[]> {
+  let rs1 = 0;
+  let rd = 0;
+  let imm12 = 0b1101;
+}
+
+def WRS_STO : RVInstI<0b000, OPC_SYSTEM, (outs), (ins), "wrs.sto", "">,
+  Sched<[]> {
+  let rs1 = 0;
+  let rd = 0;
+  let imm12 = 0b00011101;
+}
+} // Predicates = [HasStdExtZawrs]
+
 } // hasSideEffects = 1, mayLoad = 0, mayStore = 0
 
 def CSRRW : CSR_ir<0b001, "csrrw">;
Index: llvm/lib/Target/RISCV/RISCV.td
===
--- llvm/lib/Target/RISCV/RISCV.td
+++ llvm/lib/Target/RISCV/RISCV.td
@@ -461,6 +461,13 @@
AssemblerPredicate<(all_of FeatureStdExtZtso),
"'Ztso' (Memory Model - Total Store Order)">;
 
+def FeatureStdExtZawrs
+: SubtargetFeature<"experimental-zawrs", "HasStdExtZawrs", "true",
+   "'Zawrs' (Wait on Reservation Set)">;
+def HasStdExtZawrs : Predicate<"Subtarget->hasStdExtZawrs()">,
+   AssemblerPredicate<(all_of FeatureStdExtZawrs),
+   "'Zawrs' (Wait on Reservation Set)">;
+
 // Feature32Bit exists to mark CPUs that support RV32 to distinquish them from
 // tuning CPU names.
 def Feature32Bit
Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -114,6 +114,7 @@
 {"zbt", RISCVExtensionVersion{0, 93}},
 {"zca", RISCVExtensionVersion{0, 70}},
 {"zvfh", RISCVExtensionVersion{0, 1}},
+{"zawrs", RISCVExtensionVersion{1, 0}},
 {"ztso", RISCVExtensionVersion{0, 1}},
 };
 
Index: llvm/docs/RISCVUsage.rst

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

In D133443#3795603 , @asb wrote:

> Everything that's in this patch looks good to me - it's just missing some 
> simple round-trip tests in the style of rv32zicboz-valid.s (and perhaps an 
> -invalid.s that shows a sensible error message being produced when the 
> instructions have an argument).

You had me very confused at first as I'd written the valid tests - until I 
realized they'd been lost in the rebase.  Updated patch forthcoming.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133443/new/

https://reviews.llvm.org/D133443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Alex Bradbury via Phabricator via cfe-commits]

asb requested changes to this revision.
asb added a comment.
This revision now requires changes to proceed.

Everything that's in this patch looks good to me - it's just missing some 
simple round-trip tests in the style of rv32zicboz-valid.s (and perhaps an 
-invalid.s that shows a sensible error message being produced when the 
instructions have an argument).

In terms of which *.td file to edit - RISCVInstrInfoA.td is another 
possibility. The plan had been that for the `z?foo` extensions, the letter 
after the z indicated which of the top-level extensions it was most relevant to 
- e.g. the zc* extensions (ztso predated this idea). I don't mind much either 
way - I'm happy with it in either location.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133443/new/

https://reviews.llvm.org/D133443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Philip Reames via Phabricator via cfe-commits]

reames updated this revision to Diff 460447.
reames added a comment.

Add docs.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133443/new/

https://reviews.llvm.org/D133443

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/docs/RISCVUsage.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfo.td
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s

Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/attribute-arch.s
@@ -197,5 +197,8 @@
 .attribute arch, "rv32izca0p70"
 # CHECK: attribute  5, "rv32i2p0_zca0p70"
 
+.attribute arch, "rv32izawrs1p0"
+# CHECK: attribute  5, "rv32i2p0_zawrs1p0"
+
 .attribute arch, "rv32iztso0p1"
 # CHECK: attribute  5, "rv32i2p0_ztso0p1"
Index: llvm/test/CodeGen/RISCV/attributes.ll
===
--- llvm/test/CodeGen/RISCV/attributes.ll
+++ llvm/test/CodeGen/RISCV/attributes.ll
@@ -84,6 +84,7 @@
 ; RUN: llc -mtriple=riscv64 -mattr=+zicbom %s -o - | FileCheck --check-prefix=RV64ZICBOM %s
 ; RUN: llc -mtriple=riscv64 -mattr=+zicboz %s -o - | FileCheck --check-prefix=RV64ZICBOZ %s
 ; RUN: llc -mtriple=riscv64 -mattr=+zicbop %s -o - | FileCheck --check-prefix=RV64ZICBOP %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zawrs %s -o - | FileCheck --check-prefix=RV64ZAWRS %s
 ; RUN: llc -mtriple=riscv64 -mattr=+experimental-ztso %s -o - | FileCheck --check-prefix=RV64ZTSO %s
 
 ; RV32M: .attribute 5, "rv32i2p0_m2p0"
@@ -170,6 +171,7 @@
 ; RV64COMBINEINTOZKS: .attribute 5, "rv64i2p0_zbkb1p0_zbkc1p0_zbkx1p0_zks1p0_zksed1p0_zksh1p0"
 ; RV64ZICBOM: .attribute 5, "rv64i2p0_zicbom1p0"
 ; RV64ZICBOZ: .attribute 5, "rv64i2p0_zicboz1p0"
+; RV64ZAWRS: .attribute 5, "rv64i2p0_zawrs1p0"
 ; RV64ZICBOP: .attribute 5, "rv64i2p0_zicbop1p0"
 ; RV64ZTSO: .attribute 5, "rv64i2p0_ztso0p1"
 
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -92,6 +92,7 @@
   bool HasStdExtZicboz = false;
   bool HasStdExtZicbop = false;
   bool HasStdExtZmmul = false;
+  bool HasStdExtZawrs = false;
   bool HasStdExtZtso = false;
   bool HasRV32 = false;
   bool HasRV64 = false;
@@ -192,6 +193,7 @@
   bool hasStdExtZicbom() const { return HasStdExtZicbom; }
   bool hasStdExtZicboz() const { return HasStdExtZicboz; }
   bool hasStdExtZicbop() const { return HasStdExtZicbop; }
+  bool hasStdExtZawrs() const { return HasStdExtZawrs; }
   bool hasStdExtZmmul() const { return HasStdExtZmmul; }
   bool hasStdExtZtso() const { return HasStdExtZtso; }
   bool is64Bit() const { return HasRV64; }
Index: llvm/lib/Target/RISCV/RISCVInstrInfo.td
===
--- llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -705,6 +705,23 @@
   let rd = 0;
   let imm12 = 0b1100;
 }
+
+let Predicates = [HasStdExtZawrs] in {
+def WRS_NTO : RVInstI<0b000, OPC_SYSTEM, (outs), (ins), "wrs.nto", "">,
+  Sched<[]> {
+  let rs1 = 0;
+  let rd = 0;
+  let imm12 = 0b1101;
+}
+
+def WRS_STO : RVInstI<0b000, OPC_SYSTEM, (outs), (ins), "wrs.sto", "">,
+  Sched<[]> {
+  let rs1 = 0;
+  let rd = 0;
+  let imm12 = 0b00011101;
+}
+} // Predicates = [HasStdExtZawrs]
+
 } // hasSideEffects = 1, mayLoad = 0, mayStore = 0
 
 def CSRRW : CSR_ir<0b001, "csrrw">;
Index: llvm/lib/Target/RISCV/RISCV.td
===
--- llvm/lib/Target/RISCV/RISCV.td
+++ llvm/lib/Target/RISCV/RISCV.td
@@ -461,6 +461,13 @@
AssemblerPredicate<(all_of FeatureStdExtZtso),
"'Ztso' (Memory Model - Total Store Order)">;
 
+def FeatureStdExtZawrs
+: SubtargetFeature<"experimental-zawrs", "HasStdExtZawrs", "true",
+   "'Zawrs' (Wait on Reservation Set)">;
+def HasStdExtZawrs : Predicate<"Subtarget->hasStdExtZawrs()">,
+   AssemblerPredicate<(all_of FeatureStdExtZawrs),
+   "'Zawrs' (Wait on Reservation Set)">;
+
 // Feature32Bit exists to mark CPUs that support RV32 to distinquish them from
 // tuning CPU names.
 def Feature32Bit
Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -114,6 +114,7 @@
 {"zbt", RISCVExtensionVersion{0, 93}},
 {"zca", RISCVExtensionVersion{0, 70}},
 {"zvfh", RISCVExtensionVersion{0, 1}},
+{"zawrs", RISCVExtensionVersion{1, 0}},
 {"ztso", RISCVExtensionVersion{0, 1}},
 };
 
Index: llvm/docs/RISCVUsage.rst

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

In D133443#3792627 , @asb wrote:

> I think the summary of our discussion on this was:
>
> - The versioning confusion is unfortunate - ideally there would be discussion 
> elsewhere at RVI on improving the situation (either ELF attributes to 
> indicate extensions are experimental, or making that unnecessary via never 
> using 1.0 until something is ratified)
> - But the above isn't a blocker to merging. As the extension is gated by the 
> experimental flag, even if there are last minute changes the impact on users 
> should be minimal / non-existent.

Matches my takeaway.  I'm going to rebase this and add in a doc change which 
clearly notes the release candidate bit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133443/new/

https://reviews.llvm.org/D133443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

I think the summary of our discussion on this was:

- The versioning confusion is unfortunate - ideally there would be discussion 
elsewhere at RVI on improving the situation (either ELF attributes to indicate 
extensions are experimental, or making that unnecessary via never using 1.0 
until something is ratified)
- But the above isn't a blocker to merging. As the extension is gated by the 
experimental flag, even if there are last minute changes the impact on users 
should be minimal / non-existent.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133443/new/

https://reviews.llvm.org/D133443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

In D133443#3790420 , @asb wrote:

> The change in set of instructions without changing the version number is 
> concerning - do you know anyone involved in that group? It would be good to 
> feedback the difficulties this can cause for us. It's also not clear if there 
> might be changes again during ratification without changing the version 
> number, which wouldn't be ideal (though probably just about livable given 
> it's marked as experimental).

I agree that the incompatible change is highly sub-optimal.  I've asked around 
a bit on the status here, but nothing I would consider "official" or would want 
to quote someone on.  My overall take is that we probably should take this as 
experimental, but only with the understanding that it may change in 
incompatible ways.

> Let's discuss briefly in the meeting tomorrow. Implementation-wise it all 
> seems straightforward.

Definitely.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133443/new/

https://reviews.llvm.org/D133443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

The change in set of instructions without changing the version number is 
concerning - do you know anyone involved in that group? It would be good to 
feedback the difficulties this can cause for us. It's also not clear if there 
might be changes again during ratification without changing the version number, 
which wouldn't be ideal (though probably just about livable given it's marked 
as experimental).

Let's discuss briefly in the meeting tomorrow. Implementation-wise it all seems 
straightforward.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133443/new/

https://reviews.llvm.org/D133443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Philip Reames via Phabricator via cfe-commits]

reames added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:709
+
+let Predicates = [HasStdExtZawrs] in {
+def WRS_NTO : RVInstI<0b000, OPC_SYSTEM, (outs), (ins), "wrs.nto", "">,

jrtc27 wrote:
> This doesn't really belong here, but a separate RISCVInstrInfoZawrs.td also 
> seems a little overkill... hmm
I had the same thoughts.  I'm happy to defer to reviewers' preference.  



Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:95
   bool HasStdExtZmmul = false;
+  bool HasStdExtZawrs = false;
   bool HasStdExtZtso = false;

jrtc27 wrote:
> I would say keep these sorted but this seems to be a bit of a mess...
I'm happy to sort in a separate change if you'd like.  Preferably after this 
lands.  :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133443/new/

https://reviews.llvm.org/D133443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:709
+
+let Predicates = [HasStdExtZawrs] in {
+def WRS_NTO : RVInstI<0b000, OPC_SYSTEM, (outs), (ins), "wrs.nto", "">,

This doesn't really belong here, but a separate RISCVInstrInfoZawrs.td also 
seems a little overkill... hmm



Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:95
   bool HasStdExtZmmul = false;
+  bool HasStdExtZawrs = false;
   bool HasStdExtZtso = false;

I would say keep these sorted but this seems to be a bit of a mess...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133443/new/

https://reviews.llvm.org/D133443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Philip Reames via Phabricator via cfe-commits]

reames created this revision.
reames added reviewers: palmer-dabbelt, sunshaoce, craig.topper, kito-cheng, 
jrtc27, frasercrmck, asb, luismarques.
Herald added subscribers: VincentWu, luke957, StephenFan, vkmr, jdoerfert, 
evandro, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, 
the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, shiva0217, 
niosHD, sabuasal, bollu, simoncook, johnrusso, rbar, hiraditya, arichardson, 
mcrosier.
Herald added a project: All.
reames requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD, MaskRay.
Herald added projects: clang, LLVM.

This implements the Zawrs specification as specified here: 
https://github.com/riscv/riscv-zawrs/releases/download/V1.0-rc3/Zawrs.pdf.  
Despite the 1.0 version name, this does not appear to have been ratified, so 
putting it under experimental for the moment.  I have been told that the 
current version is near final, and unlikely to change (again), but have nothing 
to cite on that.

This change adds assembly support, but does not include C language or IR 
intrinsics.  We can decide if we want them, and handle that in a separate patch.

There were two prior attempts at implementing this.

D128235  by @palmer-dabbelt implements a 
prior version of this extension.  Very annoyingly, the specification appears to 
have changed *without* a change in version number.  This patch also didn't make 
the extension experimental.

D129462  by @sunshaoce implements the current 
version, but was abandoned due to confusion with the prior.  Additionally, it's 
missing a few tests.  I took the .td file change and the valid assembly test 
from that change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133443

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfo.td
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/Zawrs-valid.s
  llvm/test/MC/RISCV/attribute-arch.s

Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/attribute-arch.s
@@ -197,5 +197,8 @@
 .attribute arch, "rv32izca0p70"
 # CHECK: attribute  5, "rv32i2p0_zca0p70"
 
+.attribute arch, "rv32izawrs1p0"
+# CHECK: attribute  5, "rv32i2p0_zawrs1p0"
+
 .attribute arch, "rv32iztso0p1"
 # CHECK: attribute  5, "rv32i2p0_ztso0p1"
Index: llvm/test/MC/RISCV/Zawrs-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/Zawrs-valid.s
@@ -0,0 +1,18 @@
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-zawrs -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+experimental-zawrs -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+experimental-zawrs < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zawrs -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+experimental-zawrs < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zawrs -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
+
+# CHECK-ASM-AND-OBJ: wrs.nto
+# CHECK-ASM: encoding: [0x73,0x00,0xd0,0x00]
+wrs.nto
+
+# CHECK-ASM-AND-OBJ: wrs.sto
+# CHECK-ASM: encoding: [0x73,0x00,0xd0,0x01]
+wrs.sto
Index: llvm/test/CodeGen/RISCV/attributes.ll
===
--- llvm/test/CodeGen/RISCV/attributes.ll
+++ llvm/test/CodeGen/RISCV/attributes.ll
@@ -84,6 +84,7 @@
 ; RUN: llc -mtriple=riscv64 -mattr=+zicbom %s -o - | FileCheck --check-prefix=RV64ZICBOM %s
 ; RUN: llc -mtriple=riscv64 -mattr=+zicboz %s -o - | FileCheck --check-prefix=RV64ZICBOZ %s
 ; RUN: llc -mtriple=riscv64 -mattr=+zicbop %s -o - | FileCheck --check-prefix=RV64ZICBOP %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zawrs %s -o - | FileCheck --check-prefix=RV64ZAWRS %s
 ; RUN: llc -mtriple=riscv64 -mattr=+experimental-ztso %s -o - | FileCheck --check-prefix=RV64ZTSO %s
 
 ; RV32M: .attribute 5, "rv32i2p0_m2p0"
@@ -170,6 +171,7 @@
 ; RV64COMBINEINTOZKS: .attribute 5, "rv64i2p0_zbkb1p0_zbkc1p0_zbkx1p0_zks1p0_zksed1p0_zksh1p0"
 ; RV64ZICBOM: .attribute 5, "rv64i2p0_zicbom1p0"
 ; RV64ZICBOZ: .attribute 5, "rv64i2p0_zicboz1p0"
+; RV64ZAWRS: .attribute 5, "rv64i2p0_zawrs1p0"
 ; RV64ZICBOP: .attribute 5, "rv64i2p0_zicbop1p0"
 ; RV64ZTSO: .attribute 5, "rv64i2p0_ztso0p1"
 
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
---