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][MC] MC layer support for the experimental zalasr extension #69685

Closed
wants to merge 3 commits into from

Conversation

mehnadnerd
Copy link
Contributor

@mehnadnerd mehnadnerd commented Oct 20, 2023

This PR implements the RISC-V Atomic Load-Acquire and Store-Release Extension (Zalasr).
See also https://github.com/mehnadnerd/riscv-zalasr.

@dtcxzyw dtcxzyw changed the title [RISCV] Support for the RISCV Zalasr extension (at least at the assembly level) [RISCV][MC] MC layer support for the experimental zalasr extension Oct 20, 2023
@dtcxzyw dtcxzyw requested review from topperc and asb October 20, 2023 09:16
Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

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

Please add some regression tests:

  • clang/test/Preprocessor/riscv-target-features.c
  • llvm/test/CodeGen/RISCV/attributes.ll
  • llvm/test/MC/RISCV/attribute-arch.s
  • llvm/test/MC/RISCV/zalasr-valid.s
  • llvm/test/MC/RISCV/zalasr-invalid.s

See also https://reviews.llvm.org/D149248.

@llvmbot
Copy link
Collaborator

llvmbot commented Oct 20, 2023

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

@llvm/pr-subscribers-llvm-support

Author: Brendan Sweeney (mehnadnerd)

Changes

This PR implements the RISC-V Atomic Load-Acquire and Store-Release Extension (Zalasr).
See also https://github.com/mehnadnerd/riscv-zalasr.


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

6 Files Affected:

  • (modified) llvm/docs/RISCVUsage.rst (+3)
  • (modified) llvm/lib/Support/RISCVISAInfo.cpp (+1)
  • (modified) llvm/lib/Target/RISCV/RISCVFeatures.td (+7)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.td (+1)
  • (added) llvm/lib/Target/RISCV/RISCVInstrInfoZalasr.td (+66)
  • (modified) llvm/unittests/Support/RISCVISAInfoTest.cpp (+1)
diff --git a/llvm/docs/RISCVUsage.rst b/llvm/docs/RISCVUsage.rst
index 6812efaeb36e0c1..f9a7d4633e0a8dc 100644
--- a/llvm/docs/RISCVUsage.rst
+++ b/llvm/docs/RISCVUsage.rst
@@ -197,6 +197,9 @@ The primary goal of experimental support is to assist in the process of ratifica
 ``experimental-zacas``
   LLVM implements the `1.0-rc1 draft specification <https://github.com/riscv/riscv-zacas/releases/tag/v1.0-rc1>`_.
 
+``experimental-zalasr``
+  LLVM implements the `most recent specification <https://github.com/mehnadnerd/riscv-zalasr>`_.
+
 ``experimental-zfbfmin``, ``experimental-zvfbfmin``, ``experimental-zvfbfwma``
   LLVM implements assembler support for the `0.8.0 draft specification <https://github.com/riscv/riscv-bfloat16/releases/tag/20230629>`_.
 
diff --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp
index 22208e2e0c2950b..847a2c6f37656ba 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -168,6 +168,7 @@ static const RISCVSupportedExtension SupportedExperimentalExtensions[] = {
     {"ssaia", RISCVExtensionVersion{1, 0}},
 
     {"zacas", RISCVExtensionVersion{1, 0}},
+    {"zalasr", RISCVExtensionVersion{1, 0}},
 
     {"zfbfmin", RISCVExtensionVersion{0, 8}},
 
diff --git a/llvm/lib/Target/RISCV/RISCVFeatures.td b/llvm/lib/Target/RISCV/RISCVFeatures.td
index 979bc0ea8c7d065..b011fb4eb4421f0 100644
--- a/llvm/lib/Target/RISCV/RISCVFeatures.td
+++ b/llvm/lib/Target/RISCV/RISCVFeatures.td
@@ -708,6 +708,13 @@ def HasStdExtZacas : Predicate<"Subtarget->hasStdExtZacas()">,
                                AssemblerPredicate<(all_of FeatureStdExtZacas),
                                "'Zacas' (Atomic Compare-And-Swap Instructions)">;
 
+def FeatureStdExtZalasr
+    : SubtargetFeature<"experimental-zalasr", "HasStdExtZalasr", "true",
+                       "'Zalasr' (Load-Acquire and Store-Release Instructions)">;
+def HasStdExtZalasr : Predicate<"Subtarget->hasStdExtZalasr()">,
+                               AssemblerPredicate<(all_of FeatureStdExtZalasr),
+                               "'Zalasr' (Load-Acquire and Store-Release Instructions)">;
+
 //===----------------------------------------------------------------------===//
 // Vendor extensions
 //===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.td b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
index 94de559b1e6e037..d4d9c303d25ca06 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -2004,6 +2004,7 @@ include "RISCVInstrInfoM.td"
 
 // Atomic
 include "RISCVInstrInfoA.td"
+include "RISCVInstrInfoZalasr.td"
 
 // Scalar FP
 include "RISCVInstrInfoF.td"
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoZalasr.td b/llvm/lib/Target/RISCV/RISCVInstrInfoZalasr.td
new file mode 100644
index 000000000000000..cb603bb60306295
--- /dev/null
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoZalasr.td
@@ -0,0 +1,66 @@
+//===-- RISCVInstrInfoZalasr.td - RISC-V 'Zalasr' 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 Zalasr (Load-Acquire
+// and Store-Release) extension
+//
+//===----------------------------------------------------------------------===//
+
+//===----------------------------------------------------------------------===//
+// Instruction class templates
+//===----------------------------------------------------------------------===//
+
+let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in
+class LAQ_r<bit aq, bit rl, bits<3> funct3, string opcodestr>
+    : RVInstRAtomic<0b00110, aq, rl, funct3, OPC_AMO,
+                    (outs GPR:$rd), (ins GPRMemZeroOffset:$rs1),
+                    opcodestr, "$rd, $rs1"> {
+  let rs2 = 0;
+}
+
+let hasSideEffects = 0, mayLoad = 0, mayStore = 1 in
+class SRL_r<bit aq, bit rl, bits<3> funct3, string opcodestr>
+    : RVInstRAtomic<0b00111, aq, rl, funct3, OPC_AMO,
+                    (outs ), (ins GPRMemZeroOffset:$rs1, GPR:$rs2),
+                    opcodestr, "$rs2, $rs1"> {
+  let rd = 0;
+}
+multiclass LAQ_r_aq_rl<bits<3> funct3, string opcodestr> {
+  def _AQ    : LAQ_r<1, 0, funct3, opcodestr # ".aq">;
+  def _AQ_RL : LAQ_r<1, 1, funct3, opcodestr # ".aqrl">;
+}
+
+multiclass SRL_r_aq_rl<bits<3> funct3, string opcodestr> {
+  def _RL    : SRL_r<0, 1, funct3, opcodestr # ".rl">;
+  def _AQ_RL : SRL_r<1, 1, funct3, opcodestr # ".aqrl">;
+}
+
+//===----------------------------------------------------------------------===//
+// Instructions
+//===----------------------------------------------------------------------===//
+
+
+let Predicates = [HasStdExtZalasr] in {
+defm LB_AQ : LAQ_r_aq_rl<0b000, "lb">;
+defm LH_AQ : LAQ_r_aq_rl<0b001, "lh">;
+defm LW_AQ : LAQ_r_aq_rl<0b010, "lw">;
+defm SB_RL : SRL_r_aq_rl<0b000, "sb">;
+defm SH_RL : SRL_r_aq_rl<0b001, "sh">;
+defm SW_RL : SRL_r_aq_rl<0b010, "sw">;
+} // Predicates = [HasStdExtZalasr]
+
+let Predicates = [HasStdExtZalasr, IsRV64] in {
+defm LD_AQ : LAQ_r_aq_rl<0b011, "ld">;
+defm SD_RL : SRL_r_aq_rl<0b011, "sd">;
+} // Predicates = [HasStdExtZalasr, IsRV64]
+
+//===----------------------------------------------------------------------===//
+// Pseudo-instructions and codegen patterns
+//===----------------------------------------------------------------------===//
+
+// Future work: Work out mapping with leading/trailing fences, &c
diff --git a/llvm/unittests/Support/RISCVISAInfoTest.cpp b/llvm/unittests/Support/RISCVISAInfoTest.cpp
index 885ec88f8f697d0..ae599991d7a6d81 100644
--- a/llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -733,6 +733,7 @@ Experimental extensions
     zicfilp             0.2       This is a long dummy description
     zicond              1.0
     zacas               1.0
+    zalasr              1.0
     zfbfmin             0.8
     ztso                0.1
     zvbb                1.0

@asb
Copy link
Contributor

asb commented Oct 20, 2023

Zalasr is using the namespace reserved for standard extensions, but doesn't seem to be being developed under github.com/riscv - why is that? Is the repo going to get moved to that org? I see it is an official in-development extension based on your riscv-opcodes PR, but it would be great to remove that ambiguity.

@wangpc-pp
Copy link
Contributor

If we are going to support Zalasr, can we consider #67108 too?

@mehnadnerd
Copy link
Contributor Author

mehnadnerd commented Oct 20, 2023

Zalasr is using the namespace reserved for standard extensions, but doesn't seem to be being developed under github.com/riscv - why is that? Is the repo going to get moved to that org? I see it is an official in-development extension based on your riscv-opcodes PR, but it would be great to remove that ambiguity.

I'm honestly not entirely sure--I think it will be moved into there eventually, but I am unsure of the process.

@llvmbot llvmbot added clang Clang issues not falling into any other category mc Machine (object) code labels Oct 20, 2023
@mehnadnerd
Copy link
Contributor Author

I've updated it to add regression tests.

Copy link

github-actions bot commented Dec 18, 2023

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

const Instruction *I) const {
if (Subtarget.hasStdExtZalasr()) {
return false;
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

No else after return

@mehnadnerd
Copy link
Contributor Author

Still needs tests for the lowering.

let IsAtomicOrderingSequentiallyConsistent = 1;
}

// An atomic store operation that doesn't actually need to be atomic on RISCV.
Copy link
Collaborator

Choose a reason for hiding this comment

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

RISCV -> RISC-V

def : LdPat<atomic_load_8, LB>;
def : LdPat<atomic_load_16, LH>;
def : LdPat<atomic_load_32, LW>;
def : LdPat<relaxed_load<atomic_load_8>, LB>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is an MC patch as the title says, we shouldn't be touching any isel patterns.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was originally an MC patch but I am adding isel support as well. What should I change the title to?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We usually want MC and CodeGen changes as separate PRs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh. I will separate it out then.

@@ -1120,6 +1121,14 @@
// RUN: -o - | FileCheck --check-prefix=CHECK-SMEPMP-EXT %s
// CHECK-SMEPMP-EXT: __riscv_smepmp 1000000{{$}}

// RUN: %clang --target=riscv32 -menable-experimental-extensions \
// RUN: -march=rv32i_zalasr1p0 -x c -E -dM %s \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Indent line continuations 2 spaces and drop -x c. This was recently updated in the reset of the file.

@@ -228,6 +228,9 @@ The primary goal of experimental support is to assist in the process of ratifica
``experimental-zacas``
LLVM implements the `1.0-rc1 draft specification <https://github.com/riscv/riscv-zacas/releases/tag/v1.0-rc1>`_.

``experimental-zalasr``
LLVM implements the `most recent specification <https://github.com/mehnadnerd/riscv-zalasr>`_.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we nail this down to a version or a date? "most recent" is wrong if that repo changes.

: SubtargetFeature<"experimental-zalasr", "HasStdExtZalasr", "true",
"'Zalasr' (Load-Acquire and Store-Release Instructions)">;
def HasStdExtZalasr : Predicate<"Subtarget->hasStdExtZalasr()">,
AssemblerPredicate<(all_of FeatureStdExtZalasr),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Align AssemblerPredicate with Predicate on the previous line. This file was recently reformatted.

// Pseudo-instructions and codegen patterns
//===----------------------------------------------------------------------===//

class PatLAQ<SDPatternOperator OpNode, RVInst Inst, ValueType vt = XLenVT>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't add patterns in MC patch.

@mehnadnerd
Copy link
Contributor Author

Closing because superseded by #79911.

@mehnadnerd mehnadnerd closed this Jan 29, 2024
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.

6 participants