From 0e8bc8f590d6b87e63eb3c3de246681c154a5819 Mon Sep 17 00:00:00 2001 From: WANG Xuerui Date: Thu, 31 Oct 2024 20:06:19 +0800 Subject: [PATCH] [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 | 18 +++++++---- .../MCTargetDesc/LoongArchBaseInfo.h | 30 +++++++++++++++++++ .../MC/LoongArch/Basic/Integer/invalid64.s | 11 +++++++ 5 files changed, 67 insertions(+), 15 deletions(-) diff --git a/llvm/lib/Target/LoongArch/AsmParser/LoongArchAsmParser.cpp b/llvm/lib/Target/LoongArch/AsmParser/LoongArchAsmParser.cpp index cfbff3e91b0c5..96eb8b1b0528a 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::AMCAS_B && Opc <= LoongArch::AMCAS__DB_W) { + if (LoongArchII::isSubjectToAMORdConstraint(MCID.TSFlags)) { + const bool IsAMCAS = LoongArchII::isAMCAS(MCID.TSFlags); 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; - } else if (Opc >= LoongArch::AMADD_D && Opc <= LoongArch::AMXOR_W) { - MCRegister Rd = Inst.getOperand(0).getReg(); - MCRegister Rk = Inst.getOperand(1).getReg(); - MCRegister Rj = Inst.getOperand(2).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 6ffc8823baee0..eee297d2e2d91 100644 --- a/llvm/lib/Target/LoongArch/LoongArchInstrFormats.td +++ b/llvm/lib/Target/LoongArch/LoongArchInstrFormats.td @@ -32,6 +32,14 @@ class LAInst op> } // hasSideEffects = 0, mayLoad = 0, mayStore = 1 let hasSideEffects = 0, mayLoad = 1, mayStore = 1, - Constraints = "@earlyclobber $rd" in class AM_3R op> + IsSubjectToAMORdConstraint = 1 in { +class AM_3R op> : Fmt3R; + "$rd, $rk, $rj"> { + let Constraints = "@earlyclobber $rd"; +} -let hasSideEffects = 0, mayLoad = 1, mayStore = 1, - Constraints = - "@earlyclobber $rd_wb, $rd_wb = $rd" in class AMCAS_3R op> +class AMCAS_3R op> : Fmt3R; + "$rd, $rk, $rj"> { + let Constraints = "@earlyclobber $rd_wb, $rd_wb = $rd"; + let IsAMCAS = 1; +} +} // hasSideEffects = 0, mayLoad = 1, mayStore = 1, + // IsSubjectToAMORdConstraint = 1 let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in { class LLBase op> diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.h b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchBaseInfo.h index 1a12fb492a60f..bd63c5edeabca 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 1c1c658ad440f..0d81ddd6763b5 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