[llvm] [LoongArch] Record the special AMO operand constraint with TableGen (PR #114398)
WÁNG Xuěruì via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 31 23:56:46 PDT 2024
https://github.com/xen0n updated https://github.com/llvm/llvm-project/pull/114398
>From e20d353faf4e109fa0d4559238e6c72ba4af9562 Mon Sep 17 00:00:00 2001
From: WANG Xuerui <git at xen0n.name>
Date: Fri, 1 Nov 2024 14:02:58 +0800
Subject: [PATCH 1/2] [LoongArch][NFC] Fix the operand constraint of AMCAS
instructions
The `rd` operand of AMCAS instructions is both read and written, because
of the nature of compare-and-swap operations, but currently it is not
declared as such. Fix it for upcoming codegen enablement changes. No
functional change.
While at it, restore vertical alignment for the definition lines.
Suggested-by: tangaac <tangyan01 at loongson.cn>
Link: https://github.com/llvm/llvm-project/pull/114398#discussion_r1825362676
---
.../AsmParser/LoongArchAsmParser.cpp | 6 ++++++
.../Target/LoongArch/LoongArchInstrInfo.td | 21 ++++++++++++-------
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/Target/LoongArch/AsmParser/LoongArchAsmParser.cpp b/llvm/lib/Target/LoongArch/AsmParser/LoongArchAsmParser.cpp
index b4b19caed8999e..908d77c3e6b583 100644
--- a/llvm/lib/Target/LoongArch/AsmParser/LoongArchAsmParser.cpp
+++ b/llvm/lib/Target/LoongArch/AsmParser/LoongArchAsmParser.cpp
@@ -1568,6 +1568,12 @@ unsigned LoongArchAsmParser::checkTargetMatchPredicate(MCInst &Inst) {
MCRegister Rj = Inst.getOperand(2).getReg();
if ((Rd == Rk || Rd == Rj) && Rd != LoongArch::R0)
return Match_RequiresAMORdDifferRkRj;
+ } else if (Opc >= LoongArch::AMCAS_B && Opc <= LoongArch::AMCAS__DB_W) {
+ MCRegister Rd = Inst.getOperand(0).getReg();
+ MCRegister Rk = Inst.getOperand(2).getReg();
+ MCRegister Rj = Inst.getOperand(3).getReg();
+ if ((Rd == Rk || Rd == Rj) && Rd != LoongArch::R0)
+ return Match_RequiresAMORdDifferRkRj;
}
break;
case LoongArch::PseudoLA_TLS_DESC:
diff --git a/llvm/lib/Target/LoongArch/LoongArchInstrInfo.td b/llvm/lib/Target/LoongArch/LoongArchInstrInfo.td
index ccdd5165728231..8a9316f1133c9b 100644
--- a/llvm/lib/Target/LoongArch/LoongArchInstrInfo.td
+++ b/llvm/lib/Target/LoongArch/LoongArchInstrInfo.td
@@ -715,6 +715,11 @@ class AM_3R<bits<32> op>
: Fmt3R<op, (outs GPR:$rd), (ins GPR:$rk, GPRMemAtomic:$rj),
"$rd, $rk, $rj">;
+let hasSideEffects = 0, mayLoad = 1, mayStore = 1, Constraints = "$rd = $rd_wb" in
+class AMCAS_3R<bits<32> op>
+ : Fmt3R<op, (outs GPR:$rd_wb), (ins GPR:$rd, GPR:$rk, GPRMemAtomic:$rj),
+ "$rd, $rk, $rj">;
+
let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in {
class LLBase<bits<32> op>
: Fmt2RI14<op, (outs GPR:$rd), (ins GPR:$rj, simm14_lsl2:$imm14),
@@ -1024,14 +1029,14 @@ def AMMAX__DB_WU : AM_3R<0x38700000>;
def AMMAX__DB_DU : AM_3R<0x38708000>;
def AMMIN__DB_WU : AM_3R<0x38710000>;
def AMMIN__DB_DU : AM_3R<0x38718000>;
-def AMCAS_B : AM_3R<0x38580000>;
-def AMCAS_H : AM_3R<0x38588000>;
-def AMCAS_W : AM_3R<0x38590000>;
-def AMCAS_D : AM_3R<0x38598000>;
-def AMCAS__DB_B : AM_3R<0x385a0000>;
-def AMCAS__DB_H : AM_3R<0x385a8000>;
-def AMCAS__DB_W : AM_3R<0x385b0000>;
-def AMCAS__DB_D : AM_3R<0x385b8000>;
+def AMCAS_B : AMCAS_3R<0x38580000>;
+def AMCAS_H : AMCAS_3R<0x38588000>;
+def AMCAS_W : AMCAS_3R<0x38590000>;
+def AMCAS_D : AMCAS_3R<0x38598000>;
+def AMCAS__DB_B : AMCAS_3R<0x385a0000>;
+def AMCAS__DB_H : AMCAS_3R<0x385a8000>;
+def AMCAS__DB_W : AMCAS_3R<0x385b0000>;
+def AMCAS__DB_D : AMCAS_3R<0x385b8000>;
def LL_D : LLBase<0x22000000>;
def SC_D : SCBase<0x23000000>;
def SC_Q : SCBase_128<0x38570000>;
>From 4f1644e7945aebc58ad1e5232a90b86c84bea727 Mon Sep 17 00:00:00 2001
From: WANG Xuerui <git at xen0n.name>
Date: Thu, 31 Oct 2024 20:06:19 +0800
Subject: [PATCH 2/2] [LoongArch] Record the special AMO operand constraint
with TableGen
The LoongArch Reference Manual says that the 3-register atomic memory
operations cannot have their rd equal to either rj or rk [^1], and
both GNU as and LLVM IAS enforce the constraint for non-zero rd.
However, currently LoongArch AsmParser is checking for the opcode with
a direct numerical comparison on the opcode, which is enum-typed: the
fact that all AMO insns have adjacent numerical values is merely a
coincidence, and it is better to not rely on the current TableGen
implementation behavior.
Instead, start to leverage the target-specific flags field of
MCInstrDesc, and record the constraint with TableGen, so we can stop
treating the opcode value as number. In doing so, we also have to mark
whether the instruction is AMCAS, because the operand index of rj and
rk for the AMCAS instructions is different.
While documenting the new flag, it was found that v1.10 of the Manual
did not specify the similar constraint for the AMCAS instructions.
Experiments were done on a Loongson 3A6000 (LA664 uarch) and it turned
out that at least AMCAS will still signal INE with `rd == rj`. The
`rd == rk` case should be a no-op according to the semantics, but as it
is meaningless to perform CAS with the "old value" same as the "new
value", it is not worth special-casing. So the current behavior of also
enforcing the constraint for AMCAS is kept.
[^1]: if `rd == rj` an INE would be signaled; if `rd == rk` it is UB.
---
.../AsmParser/LoongArchAsmParser.cpp | 15 ++++------
.../Target/LoongArch/LoongArchInstrFormats.td | 8 +++++
.../Target/LoongArch/LoongArchInstrInfo.td | 15 +++++++---
.../MCTargetDesc/LoongArchBaseInfo.h | 30 +++++++++++++++++++
.../MC/LoongArch/Basic/Integer/invalid64.s | 11 +++++++
5 files changed, 66 insertions(+), 13 deletions(-)
diff --git a/llvm/lib/Target/LoongArch/AsmParser/LoongArchAsmParser.cpp b/llvm/lib/Target/LoongArch/AsmParser/LoongArchAsmParser.cpp
index 908d77c3e6b583..96eb8b1b0528ac 100644
--- a/llvm/lib/Target/LoongArch/AsmParser/LoongArchAsmParser.cpp
+++ b/llvm/lib/Target/LoongArch/AsmParser/LoongArchAsmParser.cpp
@@ -6,6 +6,7 @@
//
//===----------------------------------------------------------------------===//
+#include "MCTargetDesc/LoongArchBaseInfo.h"
#include "MCTargetDesc/LoongArchInstPrinter.h"
#include "MCTargetDesc/LoongArchMCExpr.h"
#include "MCTargetDesc/LoongArchMCTargetDesc.h"
@@ -1560,18 +1561,14 @@ bool LoongArchAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
unsigned LoongArchAsmParser::checkTargetMatchPredicate(MCInst &Inst) {
unsigned Opc = Inst.getOpcode();
+ const MCInstrDesc &MCID = MII.get(Opc);
switch (Opc) {
default:
- if (Opc >= LoongArch::AMADD_D && Opc <= LoongArch::AMXOR_W) {
+ if (LoongArchII::isSubjectToAMORdConstraint(MCID.TSFlags)) {
+ const bool IsAMCAS = LoongArchII::isAMCAS(MCID.TSFlags);
MCRegister Rd = Inst.getOperand(0).getReg();
- MCRegister Rk = Inst.getOperand(1).getReg();
- MCRegister Rj = Inst.getOperand(2).getReg();
- if ((Rd == Rk || Rd == Rj) && Rd != LoongArch::R0)
- return Match_RequiresAMORdDifferRkRj;
- } else if (Opc >= LoongArch::AMCAS_B && Opc <= LoongArch::AMCAS__DB_W) {
- MCRegister Rd = Inst.getOperand(0).getReg();
- MCRegister Rk = Inst.getOperand(2).getReg();
- MCRegister Rj = Inst.getOperand(3).getReg();
+ MCRegister Rk = Inst.getOperand(IsAMCAS ? 2 : 1).getReg();
+ MCRegister Rj = Inst.getOperand(IsAMCAS ? 3 : 2).getReg();
if ((Rd == Rk || Rd == Rj) && Rd != LoongArch::R0)
return Match_RequiresAMORdDifferRkRj;
}
diff --git a/llvm/lib/Target/LoongArch/LoongArchInstrFormats.td b/llvm/lib/Target/LoongArch/LoongArchInstrFormats.td
index 6ffc8823baee01..eee297d2e2d916 100644
--- a/llvm/lib/Target/LoongArch/LoongArchInstrFormats.td
+++ b/llvm/lib/Target/LoongArch/LoongArchInstrFormats.td
@@ -32,6 +32,14 @@ class LAInst<dag outs, dag ins, string opcstr, string opnstr,
let InOperandList = ins;
let AsmString = opcstr # "\t" # opnstr;
let Pattern = pattern;
+
+ // Target-specific instruction info and defaults
+
+ bit IsSubjectToAMORdConstraint = 0;
+ let TSFlags{0} = IsSubjectToAMORdConstraint;
+
+ bit IsAMCAS = 0;
+ let TSFlags{1} = IsAMCAS;
}
// Pseudo instructions
diff --git a/llvm/lib/Target/LoongArch/LoongArchInstrInfo.td b/llvm/lib/Target/LoongArch/LoongArchInstrInfo.td
index 8a9316f1133c9b..a683af9622a468 100644
--- a/llvm/lib/Target/LoongArch/LoongArchInstrInfo.td
+++ b/llvm/lib/Target/LoongArch/LoongArchInstrInfo.td
@@ -710,15 +710,22 @@ class STORE_2RI14<bits<32> op>
"$rd, $rj, $imm14">;
} // hasSideEffects = 0, mayLoad = 0, mayStore = 1
-let hasSideEffects = 0, mayLoad = 1, mayStore = 1, Constraints = "@earlyclobber $rd" in
+let hasSideEffects = 0, mayLoad = 1, mayStore = 1,
+ IsSubjectToAMORdConstraint = 1 in {
class AM_3R<bits<32> op>
: Fmt3R<op, (outs GPR:$rd), (ins GPR:$rk, GPRMemAtomic:$rj),
- "$rd, $rk, $rj">;
+ "$rd, $rk, $rj"> {
+ let Constraints = "@earlyclobber $rd";
+}
-let hasSideEffects = 0, mayLoad = 1, mayStore = 1, Constraints = "$rd = $rd_wb" in
class AMCAS_3R<bits<32> op>
: Fmt3R<op, (outs GPR:$rd_wb), (ins GPR:$rd, GPR:$rk, GPRMemAtomic:$rj),
- "$rd, $rk, $rj">;
+ "$rd, $rk, $rj"> {
+ let Constraints = "$rd = $rd_wb";
+ let IsAMCAS = 1;
+}
+} // hasSideEffects = 0, mayLoad = 1, mayStore = 1,
+ // IsSubjectToAMORdConstraint = 1
let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in {
class LLBase<bits<32> op>
diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.h b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.h
index 1a12fb492a60f5..bd63c5edeabca7 100644
--- a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.h
+++ b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.h
@@ -56,6 +56,36 @@ enum {
MO_DESC_CALL,
// TODO: Add more flags.
};
+
+// Target-specific flags of LAInst.
+// All definitions must match LoongArchInstrFormats.td.
+enum {
+ // Whether the instruction's rd is normally required to differ from rj and
+ // rk, in the way the 3-register atomic memory operations behave
+ // (Section 2.2.7.1 and 2.2.7.2, LoongArch Reference Manual Volume 1 v1.10;
+ // while Section 2.2.7.3 lacked similar description for the AMCAS
+ // instructions, at least the INE exception is still signaled on Loongson
+ // 3A6000 when its rd == rj).
+ //
+ // Used for generating diagnostics for assembler input that violate the
+ // constraint. As described on the manual, the covered instructions require
+ // rd != rj && rd != rk to work as intended.
+ IsSubjectToAMORdConstraintShift = 0,
+ IsSubjectToAMORdConstraintMask = 1 << IsSubjectToAMORdConstraintShift,
+
+ // Whether the instruction belongs to the AMCAS family.
+ IsAMCASShift = IsSubjectToAMORdConstraintShift + 1,
+ IsAMCASMask = 1 << IsAMCASShift,
+};
+
+/// \returns true if this instruction's rd is normally required to differ
+/// from rj and rk, in the way 3-register atomic memory operations behave.
+static inline bool isSubjectToAMORdConstraint(uint64_t TSFlags) {
+ return TSFlags & IsSubjectToAMORdConstraintMask;
+}
+
+/// \returns true if this instruction belongs to the AMCAS family.
+static inline bool isAMCAS(uint64_t TSFlags) { return TSFlags & IsAMCASMask; }
} // end namespace LoongArchII
namespace LoongArchABI {
diff --git a/llvm/test/MC/LoongArch/Basic/Integer/invalid64.s b/llvm/test/MC/LoongArch/Basic/Integer/invalid64.s
index 1c1c658ad440f8..0d81ddd6763b56 100644
--- a/llvm/test/MC/LoongArch/Basic/Integer/invalid64.s
+++ b/llvm/test/MC/LoongArch/Basic/Integer/invalid64.s
@@ -91,3 +91,14 @@ amxor.w $a0, $a1, $a0
amadd.d $a0, $a1, $a2, $a3
# CHECK: :[[#@LINE+1]]:24: error: optional integer offset must be 0
amadd.d $a0, $a1, $a2, 1
+
+## According to experiment results on real LA664 HW, the AMCAS instructions
+## are subject to the same constraint as the other 3-register atomic insns.
+## This is undocumented in v1.10 of the LoongArch Reference Manual.
+
+# CHECK: :[[#@LINE+1]]:10: error: $rd must be different from both $rk and $rj
+amcas.b $a0, $a0, $a0
+# CHECK: :[[#@LINE+1]]:10: error: $rd must be different from both $rk and $rj
+amcas.h $a0, $a0, $a1
+# CHECK: :[[#@LINE+1]]:13: error: $rd must be different from both $rk and $rj
+amcas_db.w $a0, $a1, $a0
More information about the llvm-commits
mailing list