Skip to content

[TableGen][RISCV][AArch64][GISel] Properly implement isAnyExtLoad/isSignExtLoad/isZeroExtLoad for IsAtomic in SelectionDAG. #137096

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 24, 2025
Merged

[TableGen][RISCV][AArch64][GISel] Properly implement isAnyExtLoad/isSignExtLoad/isZeroExtLoad for IsAtomic in SelectionDAG. #137096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3032131
[TableGen][RISCV][AArch64] Properly implement isAnyExtLoad/isSignExtL…
topperc Apr 24, 2025
6ab9b56
fixup! Rename atomic_load_az*
topperc Apr 24, 2025
46a24fa
fixup! fix copy paste mistake in error message.
topperc Apr 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 58 additions & 6 deletions llvm/include/llvm/Target/TargetSelectionDAG.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1852,6 +1852,13 @@ def atomic_load_sext :
let IsSignExtLoad = true;
}

/// Atomic load which any extends the excess high bits.
def atomic_load_aext :
PatFrag<(ops node:$ptr), (atomic_load node:$ptr)> {
let IsAtomic = true; // FIXME: Should be IsLoad and/or IsAtomic?
let IsAnyExtLoad = true;
}

def atomic_load_8 :
PatFrag<(ops node:$ptr),
(atomic_load node:$ptr)> {
Expand Down Expand Up @@ -1891,6 +1898,12 @@ def atomic_load_zext_16 :
let MemoryVT = i16;
}

def atomic_load_zext_32 :
PatFrag<(ops node:$ptr), (atomic_load_zext node:$ptr)> {
let IsAtomic = true; // FIXME: Should be IsLoad and/or IsAtomic?
let MemoryVT = i32;
}

def atomic_load_sext_8 :
PatFrag<(ops node:$ptr), (atomic_load_sext node:$ptr)> {
let IsAtomic = true; // FIXME: Should be IsLoad and/or IsAtomic?
Expand All @@ -1903,15 +1916,54 @@ def atomic_load_sext_16 :
let MemoryVT = i16;
}

def atomic_load_sext_32 :
PatFrag<(ops node:$ptr), (atomic_load_sext node:$ptr)> {
let IsAtomic = true; // FIXME: Should be IsLoad and/or IsAtomic?
let MemoryVT = i32;
}

def atomic_load_aext_8 :
PatFrag<(ops node:$ptr), (atomic_load_aext node:$ptr)> {
let IsAtomic = true; // FIXME: Should be IsLoad and/or IsAtomic?
let MemoryVT = i8;
}

def atomic_load_aext_16 :
PatFrag<(ops node:$ptr), (atomic_load_aext node:$ptr)> {
let IsAtomic = true; // FIXME: Should be IsLoad and/or IsAtomic?
Copy link
Contributor

Choose a reason for hiding this comment

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

There's some wonky logic in both emitters that seem to assume you only set one of these bits per pat frag. I don't think it works to set both, should probably do something about that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The SelectionDAG emitter has some checks for multiple bits being set, but I don't know if it is complete.

let MemoryVT = i16;
}

def atomic_load_aext_32 :
PatFrag<(ops node:$ptr), (atomic_load_aext node:$ptr)> {
let IsAtomic = true; // FIXME: Should be IsLoad and/or IsAtomic?
let MemoryVT = i32;
}

// Atomic load which zeroes or anyextends the high bits.
def atomic_load_az_8 : PatFrags<(ops node:$op),
[(atomic_load_8 node:$op),
(atomic_load_zext_8 node:$op)]>;
def atomic_load_azext_8 : PatFrags<(ops node:$op),
[(atomic_load_aext_8 node:$op),
(atomic_load_zext_8 node:$op)]>;

// Atomic load which zeroes or anyextends the high bits.
def atomic_load_az_16 : PatFrags<(ops node:$op),
[(atomic_load_16 node:$op),
(atomic_load_zext_16 node:$op)]>;
def atomic_load_azext_16 : PatFrags<(ops node:$op),
[(atomic_load_aext_16 node:$op),
(atomic_load_zext_16 node:$op)]>;

// Atomic load which sign extends or anyextends the high bits.
def atomic_load_asext_8 : PatFrags<(ops node:$op),
[(atomic_load_aext_8 node:$op),
(atomic_load_sext_8 node:$op)]>;

// Atomic load which sign extends or anyextends the high bits.
def atomic_load_asext_16 : PatFrags<(ops node:$op),
[(atomic_load_aext_16 node:$op),
(atomic_load_sext_16 node:$op)]>;

// Atomic load which sign extends or anyextends the high bits.
def atomic_load_asext_32 : PatFrags<(ops node:$op),
[(atomic_load_aext_32 node:$op),
(atomic_load_sext_32 node:$op)]>;

def nonext_masked_gather :
PatFrag<(ops node:$def, node:$pred, node:$ptr, node:$idx),
Expand Down
40 changes: 20 additions & 20 deletions llvm/lib/Target/AArch64/AArch64InstrAtomics.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,34 +61,34 @@ let Predicates = [HasRCPC] in {
}

// 8-bit loads
def : Pat<(seq_cst_load<atomic_load_az_8> GPR64sp:$ptr), (LDARB GPR64sp:$ptr)>;
def : Pat<(acquiring_load<atomic_load_az_8> GPR64sp:$ptr), (LDARB GPR64sp:$ptr)>;
def : Pat<(relaxed_load<atomic_load_az_8> (ro_Windexed8 GPR64sp:$Rn, GPR32:$Rm,
ro_Wextend8:$offset)),
def : Pat<(seq_cst_load<atomic_load_azext_8> GPR64sp:$ptr), (LDARB GPR64sp:$ptr)>;
def : Pat<(acquiring_load<atomic_load_azext_8> GPR64sp:$ptr), (LDARB GPR64sp:$ptr)>;
def : Pat<(relaxed_load<atomic_load_azext_8> (ro_Windexed8 GPR64sp:$Rn, GPR32:$Rm,
ro_Wextend8:$offset)),
(LDRBBroW GPR64sp:$Rn, GPR32:$Rm, ro_Wextend8:$offset)>;
def : Pat<(relaxed_load<atomic_load_az_8> (ro_Xindexed8 GPR64sp:$Rn, GPR64:$Rm,
ro_Xextend8:$offset)),
def : Pat<(relaxed_load<atomic_load_azext_8> (ro_Xindexed8 GPR64sp:$Rn, GPR64:$Rm,
ro_Xextend8:$offset)),
(LDRBBroX GPR64sp:$Rn, GPR64:$Rm, ro_Xextend8:$offset)>;
def : Pat<(relaxed_load<atomic_load_az_8> (am_indexed8 GPR64sp:$Rn,
uimm12s1:$offset)),
def : Pat<(relaxed_load<atomic_load_azext_8> (am_indexed8 GPR64sp:$Rn,
uimm12s1:$offset)),
(LDRBBui GPR64sp:$Rn, uimm12s1:$offset)>;
def : Pat<(relaxed_load<atomic_load_az_8>
def : Pat<(relaxed_load<atomic_load_azext_8>
(am_unscaled8 GPR64sp:$Rn, simm9:$offset)),
(LDURBBi GPR64sp:$Rn, simm9:$offset)>;

// 16-bit loads
def : Pat<(seq_cst_load<atomic_load_az_16> GPR64sp:$ptr), (LDARH GPR64sp:$ptr)>;
def : Pat<(acquiring_load<atomic_load_az_16> GPR64sp:$ptr), (LDARH GPR64sp:$ptr)>;
def : Pat<(relaxed_load<atomic_load_az_16> (ro_Windexed16 GPR64sp:$Rn, GPR32:$Rm,
ro_Wextend16:$extend)),
def : Pat<(seq_cst_load<atomic_load_azext_16> GPR64sp:$ptr), (LDARH GPR64sp:$ptr)>;
def : Pat<(acquiring_load<atomic_load_azext_16> GPR64sp:$ptr), (LDARH GPR64sp:$ptr)>;
def : Pat<(relaxed_load<atomic_load_azext_16> (ro_Windexed16 GPR64sp:$Rn, GPR32:$Rm,
ro_Wextend16:$extend)),
(LDRHHroW GPR64sp:$Rn, GPR32:$Rm, ro_Wextend16:$extend)>;
def : Pat<(relaxed_load<atomic_load_az_16> (ro_Xindexed16 GPR64sp:$Rn, GPR64:$Rm,
ro_Xextend16:$extend)),
def : Pat<(relaxed_load<atomic_load_azext_16> (ro_Xindexed16 GPR64sp:$Rn, GPR64:$Rm,
ro_Xextend16:$extend)),
(LDRHHroX GPR64sp:$Rn, GPR64:$Rm, ro_Xextend16:$extend)>;
def : Pat<(relaxed_load<atomic_load_az_16> (am_indexed16 GPR64sp:$Rn,
uimm12s2:$offset)),
def : Pat<(relaxed_load<atomic_load_azext_16> (am_indexed16 GPR64sp:$Rn,
uimm12s2:$offset)),
(LDRHHui GPR64sp:$Rn, uimm12s2:$offset)>;
def : Pat<(relaxed_load<atomic_load_az_16>
def : Pat<(relaxed_load<atomic_load_azext_16>
(am_unscaled16 GPR64sp:$Rn, simm9:$offset)),
(LDURHHi GPR64sp:$Rn, simm9:$offset)>;

Expand Down Expand Up @@ -591,10 +591,10 @@ let Predicates = [HasRCPC3, HasNEON] in {
// v8.4a FEAT_LRCPC2 patterns
let Predicates = [HasRCPC_IMMO, UseLDAPUR] in {
// Load-Acquire RCpc Register unscaled loads
def : Pat<(acquiring_load<atomic_load_az_8>
def : Pat<(acquiring_load<atomic_load_azext_8>
(am_unscaled8 GPR64sp:$Rn, simm9:$offset)),
(LDAPURBi GPR64sp:$Rn, simm9:$offset)>;
def : Pat<(acquiring_load<atomic_load_az_16>
def : Pat<(acquiring_load<atomic_load_azext_16>
(am_unscaled16 GPR64sp:$Rn, simm9:$offset)),
(LDAPURHi GPR64sp:$Rn, simm9:$offset)>;
def : Pat<(acquiring_load<atomic_load_32>
Expand Down
8 changes: 4 additions & 4 deletions llvm/lib/Target/BPF/BPFInstrInfo.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1342,11 +1342,11 @@ let Predicates = [BPFHasALU32] in {

let Predicates = [BPFHasLoadAcqStoreRel] in {
foreach P = [[relaxed_load<atomic_load_32>, LDW32],
[relaxed_load<atomic_load_az_16>, LDH32],
[relaxed_load<atomic_load_az_8>, LDB32],
[relaxed_load<atomic_load_azext_16>, LDH32],
[relaxed_load<atomic_load_azext_8>, LDB32],
[acquiring_load<atomic_load_32>, LDWACQ32],
[acquiring_load<atomic_load_az_16>, LDHACQ32],
[acquiring_load<atomic_load_az_8>, LDBACQ32],
[acquiring_load<atomic_load_azext_16>, LDHACQ32],
[acquiring_load<atomic_load_azext_8>, LDBACQ32],
] in {
def : Pat<(P[0] ADDRri:$addr), (P[1] ADDRri:$addr)>;
}
Expand Down
29 changes: 3 additions & 26 deletions llvm/lib/Target/RISCV/RISCVInstrInfoA.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,29 +118,6 @@ defm AMOMAXU_D : AMO_rr_aq_rl<0b11100, 0b011, "amomaxu.d">,
// Pseudo-instructions and codegen patterns
//===----------------------------------------------------------------------===//

def riscv_atomic_asextload : PatFrag<(ops node:$ptr), (atomic_load node:$ptr), [{
ISD::LoadExtType ETy = cast<AtomicSDNode>(N)->getExtensionType();
return ETy == ISD::EXTLOAD || ETy == ISD::SEXTLOAD;
}]>;

def riscv_atomic_asextload_8 : PatFrag<(ops node:$ptr),
(riscv_atomic_asextload node:$ptr)> {
let IsAtomic = true;
let MemoryVT = i8;
}

def riscv_atomic_asextload_16 : PatFrag<(ops node:$ptr),
(riscv_atomic_asextload node:$ptr)> {
let IsAtomic = true;
let MemoryVT = i16;
}

def riscv_atomic_asextload_32 : PatFrag<(ops node:$ptr),
(riscv_atomic_asextload node:$ptr)> {
let IsAtomic = true;
let MemoryVT = i32;
}

let IsAtomic = 1 in {
// An atomic load operation that does not need either acquire or release
// semantics.
Expand Down Expand Up @@ -188,8 +165,8 @@ class seq_cst_store<PatFrag base>
// any ordering. This is necessary because AtomicExpandPass has added fences to
// atomic load/stores and changed them to unordered ones.
let Predicates = [HasAtomicLdSt] in {
def : LdPat<relaxed_load<riscv_atomic_asextload_8>, LB>;
def : LdPat<relaxed_load<riscv_atomic_asextload_16>, LH>;
def : LdPat<relaxed_load<atomic_load_asext_8>, LB>;
def : LdPat<relaxed_load<atomic_load_asext_16>, LH>;

def : StPat<relaxed_store<atomic_store_8>, SB, GPR, XLenVT>;
def : StPat<relaxed_store<atomic_store_16>, SH, GPR, XLenVT>;
Expand All @@ -201,7 +178,7 @@ let Predicates = [HasAtomicLdSt, IsRV32] in {
}

let Predicates = [HasAtomicLdSt, IsRV64] in {
def : LdPat<relaxed_load<riscv_atomic_asextload_32>, LW>;
def : LdPat<relaxed_load<atomic_load_asext_32>, LW>;
def : LdPat<relaxed_load<atomic_load_64>, LD, i64>;
def : StPat<relaxed_store<atomic_store_64>, SD, GPR, i64>;
}
Expand Down
12 changes: 6 additions & 6 deletions llvm/lib/Target/RISCV/RISCVInstrInfoZalasr.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,11 @@ class PatSRL<SDPatternOperator OpNode, RVInst Inst, ValueType vt = XLenVT>
let Predicates = [HasStdExtZalasr] in {
// the sequentially consistent loads use
// .aq instead of .aqrl to match the psABI/A.7
def : PatLAQ<acquiring_load<riscv_atomic_asextload_8>, LB_AQ>;
def : PatLAQ<seq_cst_load<riscv_atomic_asextload_8>, LB_AQ>;
def : PatLAQ<acquiring_load<atomic_load_asext_8>, LB_AQ>;
def : PatLAQ<seq_cst_load<atomic_load_asext_8>, LB_AQ>;

def : PatLAQ<acquiring_load<riscv_atomic_asextload_16>, LH_AQ>;
def : PatLAQ<seq_cst_load<riscv_atomic_asextload_16>, LH_AQ>;
def : PatLAQ<acquiring_load<atomic_load_asext_16>, LH_AQ>;
def : PatLAQ<seq_cst_load<atomic_load_asext_16>, LH_AQ>;

// the sequentially consistent stores use
// .rl instead of .aqrl to match the psABI/A.7
Expand All @@ -101,8 +101,8 @@ let Predicates = [HasStdExtZalasr, IsRV32] in {
} // Predicates = [HasStdExtZalasr, IsRV64]

let Predicates = [HasStdExtZalasr, IsRV64] in {
def : PatLAQ<acquiring_load<riscv_atomic_asextload_32>, LW_AQ>;
def : PatLAQ<seq_cst_load<riscv_atomic_asextload_32>, LW_AQ>;
def : PatLAQ<acquiring_load<atomic_load_asext_32>, LW_AQ>;
def : PatLAQ<seq_cst_load<atomic_load_asext_32>, LW_AQ>;

def : PatLAQ<acquiring_load<atomic_load_64>, LD_AQ>;
def : PatLAQ<seq_cst_load<atomic_load_64>, LD_AQ>;
Expand Down
33 changes: 23 additions & 10 deletions llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -910,7 +910,7 @@ std::string TreePredicateFn::getPredCode() const {

if (!isLoad() && !isStore() && !isAtomic() && getMemoryVT())
PrintFatalError(getOrigPatFragRecord()->getRecord()->getLoc(),
"MemoryVT requires IsLoad or IsStore");
"MemoryVT requires IsLoad or IsStore or IsAtomic");

if (!isLoad() && !isStore()) {
if (isUnindexed())
Expand All @@ -937,11 +937,10 @@ std::string TreePredicateFn::getPredCode() const {
if (isNonExtLoad())
PrintFatalError(getOrigPatFragRecord()->getRecord()->getLoc(),
"IsNonExtLoad requires IsLoad");
if (isAnyExtLoad())
PrintFatalError(getOrigPatFragRecord()->getRecord()->getLoc(),
"IsAnyExtLoad requires IsLoad");

if (!isAtomic()) {
if (isAnyExtLoad())
PrintFatalError(getOrigPatFragRecord()->getRecord()->getLoc(),
"IsAnyExtLoad requires IsLoad or IsAtomic");
if (isSignExtLoad())
PrintFatalError(getOrigPatFragRecord()->getRecord()->getLoc(),
"IsSignExtLoad requires IsLoad or IsAtomic");
Expand Down Expand Up @@ -970,8 +969,9 @@ std::string TreePredicateFn::getPredCode() const {
if (getMemoryVT() == nullptr && !isAtomicOrderingMonotonic() &&
getAddressSpaces() == nullptr &&
// FIXME: Should atomic loads be IsLoad, IsAtomic, or both?
!isZeroExtLoad() && !isSignExtLoad() && !isAtomicOrderingAcquire() &&
!isAtomicOrderingRelease() && !isAtomicOrderingAcquireRelease() &&
!isAnyExtLoad() && !isZeroExtLoad() && !isSignExtLoad() &&
!isAtomicOrderingAcquire() && !isAtomicOrderingRelease() &&
!isAtomicOrderingAcquireRelease() &&
!isAtomicOrderingSequentiallyConsistent() &&
!isAtomicOrderingAcquireOrStronger() &&
!isAtomicOrderingReleaseOrStronger() &&
Expand Down Expand Up @@ -1075,9 +1075,22 @@ std::string TreePredicateFn::getPredCode() const {
"if (isReleaseOrStronger(cast<AtomicSDNode>(N)->getMergedOrdering())) "
"return false;\n";

// TODO: Handle atomic sextload/zextload normally when ATOMIC_LOAD is removed.
if (isAtomic() && (isZeroExtLoad() || isSignExtLoad()))
Code += "return false;\n";
if (isAtomic()) {
if ((isAnyExtLoad() + isSignExtLoad() + isZeroExtLoad()) > 1)
PrintFatalError(getOrigPatFragRecord()->getRecord()->getLoc(),
"IsAnyExtLoad, IsSignExtLoad, and IsZeroExtLoad are "
"mutually exclusive");

if (isAnyExtLoad())
Code += "if (cast<AtomicSDNode>(N)->getExtensionType() != ISD::EXTLOAD) "
"return false;\n";
if (isSignExtLoad())
Code += "if (cast<AtomicSDNode>(N)->getExtensionType() != ISD::SEXTLOAD) "
"return false;\n";
if (isZeroExtLoad())
Code += "if (cast<AtomicSDNode>(N)->getExtensionType() != ISD::ZEXTLOAD) "
"return false;\n";
}

if (isLoad() || isStore()) {
StringRef SDNodeName = isLoad() ? "LoadSDNode" : "StoreSDNode";
Expand Down
3 changes: 2 additions & 1 deletion llvm/utils/TableGen/GlobalISelEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,8 @@ Expected<InstructionMatcher &> GlobalISelEmitter::addBuiltinPredicates(
0, MemoryVsLLTSizePredicateMatcher::EqualTo, 0);
return InsnMatcher;
}
if (Predicate.isLoad() && Predicate.isAnyExtLoad()) {
if ((Predicate.isLoad() || Predicate.isAtomic()) &&
Predicate.isAnyExtLoad()) {
InsnMatcher.addPredicate<MemoryVsLLTSizePredicateMatcher>(
0, MemoryVsLLTSizePredicateMatcher::LessThan, 0);
return InsnMatcher;
Expand Down
Loading