[CodeGen] Improve getLoadExtAction and friends#181104
Open
QuantumSegfault wants to merge 3 commits intollvm:mainfrom
Open
[CodeGen] Improve getLoadExtAction and friends#181104QuantumSegfault wants to merge 3 commits intollvm:mainfrom
getLoadExtAction and friends#181104QuantumSegfault wants to merge 3 commits intollvm:mainfrom
Conversation
richer `getLoadAction`
QuantumSegfault
temporarily deployed
to
main-branch-only
GitHub Actions
Inactive
llvmbot
added
backend:AArch64
backend:X86
llvm:codegen
llvm:SelectionDAG
llvmbot
temporarily deployed
to
main-branch-only
GitHub Actions
Inactive
llvmbot
temporarily deployed
to
main-branch-only
GitHub Actions
Inactive
llvmbot
temporarily deployed
to
main-branch-only
GitHub Actions
Inactive
llvmbot
temporarily deployed
to
main-branch-only
GitHub Actions
Inactive
Member
|
@llvm/pr-subscribers-backend-x86 @llvm/pr-subscribers-backend-aarch64 Author: Demetrius Kanios (QuantumSegfault) ChangesAlternative approach to the same goals as #162407 This takes This is fully backwards compatible, with the existing setLoadExtAction working as before. But this allows targets to override the query to make more use of the information. Patch is 45.07 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/181104.diff 10 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/BasicTTIImpl.h b/llvm/include/llvm/CodeGen/BasicTTIImpl.h
index 21afcbefdf719..132dc9d18845d 100644
--- a/llvm/include/llvm/CodeGen/BasicTTIImpl.h
+++ b/llvm/include/llvm/CodeGen/BasicTTIImpl.h
@@ -28,6 +28,7 @@
#include "llvm/Analysis/TargetTransformInfoImpl.h"
#include "llvm/Analysis/ValueTracking.h"
#include "llvm/CodeGen/ISDOpcodes.h"
+#include "llvm/CodeGen/MachineMemOperand.h"
#include "llvm/CodeGen/TargetLowering.h"
#include "llvm/CodeGen/TargetSubtargetInfo.h"
#include "llvm/CodeGen/ValueTypes.h"
@@ -1261,10 +1262,41 @@ class BasicTTIImplBase : public TargetTransformInfoImplCRTPBase<T> {
EVT ExtVT = EVT::getEVT(Dst);
EVT LoadVT = EVT::getEVT(Src);
unsigned LType =
- ((Opcode == Instruction::ZExt) ? ISD::ZEXTLOAD : ISD::SEXTLOAD);
- if (DstLT.first == SrcLT.first &&
- TLI->isLoadExtLegal(LType, ExtVT, LoadVT))
- return 0;
+ ((Opcode == Instruction::ZExt) ? ISD::ZEXTLOAD : ISD::SEXTLOAD);
+ if (I) {
+ if (auto *LI = dyn_cast<LoadInst>(I->getOperand(0))) {
+ if (DstLT.first == SrcLT.first &&
+ TLI->isLoadLegal(
+ ExtVT, LoadVT, LI->getAlign(),
+ TLI->getLoadMemOperandFlags(*LI, DL),
+ LI->getPointerAddressSpace(), LType, false))
+ return 0;
+ } else if (auto *II = dyn_cast<IntrinsicInst>(I->getOperand(0))) {
+ switch (II->getIntrinsicID()) {
+ case Intrinsic::masked_load: {
+ Type *PtrType = II->getArgOperand(0)->getType();
+ assert(PtrType->isPointerTy());
+
+ auto MMOFlags = MachineMemOperand::MOLoad;
+ if (I->hasMetadata(LLVMContext::MD_nontemporal))
+ MMOFlags |= MachineMemOperand::MONonTemporal;
+ if (I->hasMetadata(LLVMContext::MD_invariant_load))
+ MMOFlags |= MachineMemOperand::MOInvariant;
+
+ if (DstLT.first == SrcLT.first &&
+ TLI->isLoadLegal(ExtVT, LoadVT,
+ II->getParamAlign(0).valueOrOne(), MMOFlags,
+ PtrType->getPointerAddressSpace(), LType,
+ false))
+ return 0;
+
+ break;
+ }
+ default:
+ break;
+ }
+ }
+ }
}
break;
case Instruction::AddrSpaceCast:
@@ -1556,7 +1588,9 @@ class BasicTTIImplBase : public TargetTransformInfoImplCRTPBase<T> {
if (Opcode == Instruction::Store)
LA = getTLI()->getTruncStoreAction(LT.second, MemVT);
else
- LA = getTLI()->getLoadExtAction(ISD::EXTLOAD, LT.second, MemVT);
+ LA = getTLI()->getLoadAction(LT.second, MemVT, Alignment,
+ MachineMemOperand::Flags::MOLoad,
+ AddressSpace, ISD::EXTLOAD, false);
if (LA != TargetLowering::Legal && LA != TargetLowering::Custom) {
// This is a vector load/store for some illegal type that is scalarized.
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index ada4ffd3bcc89..1bdf87e92edcc 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -31,6 +31,7 @@
#include "llvm/CodeGen/ISDOpcodes.h"
#include "llvm/CodeGen/LibcallLoweringInfo.h"
#include "llvm/CodeGen/LowLevelTypeUtils.h"
+#include "llvm/CodeGen/MachineMemOperand.h"
#include "llvm/CodeGen/MachineRegisterInfo.h"
#include "llvm/CodeGen/RuntimeLibcallUtil.h"
#include "llvm/CodeGen/SelectionDAG.h"
@@ -1511,49 +1512,29 @@ class LLVM_ABI TargetLoweringBase {
/// Return how this load with extension should be treated: either it is legal,
/// needs to be promoted to a larger size, needs to be expanded to some other
/// code sequence, or the target has a custom expander for it.
- LegalizeAction getLoadExtAction(unsigned ExtType, EVT ValVT,
- EVT MemVT) const {
- if (ValVT.isExtended() || MemVT.isExtended()) return Expand;
- unsigned ValI = (unsigned) ValVT.getSimpleVT().SimpleTy;
- unsigned MemI = (unsigned) MemVT.getSimpleVT().SimpleTy;
- assert(ExtType < ISD::LAST_LOADEXT_TYPE && ValI < MVT::VALUETYPE_SIZE &&
- MemI < MVT::VALUETYPE_SIZE && "Table isn't big enough!");
- unsigned Shift = 4 * ExtType;
- return (LegalizeAction)((LoadExtActions[ValI][MemI] >> Shift) & 0xf);
- }
+ virtual LegalizeAction getLoadAction(EVT ValVT, EVT MemVT, Align Alignment,
+ MachineMemOperand::Flags MMOFlags,
+ unsigned AddrSpace, unsigned ExtType,
+ bool Atomic) const;
/// Return true if the specified load with extension is legal on this target.
- bool isLoadExtLegal(unsigned ExtType, EVT ValVT, EVT MemVT) const {
- return getLoadExtAction(ExtType, ValVT, MemVT) == Legal;
+ bool isLoadLegal(EVT ValVT, EVT MemVT, Align Alignment,
+ MachineMemOperand::Flags MMOFlags, unsigned AddrSpace,
+ unsigned ExtType, bool Atomic) const {
+ return getLoadAction(ValVT, MemVT, Alignment, MMOFlags, AddrSpace, ExtType,
+ Atomic) == Legal;
}
/// Return true if the specified load with extension is legal or custom
/// on this target.
- bool isLoadExtLegalOrCustom(unsigned ExtType, EVT ValVT, EVT MemVT) const {
- return getLoadExtAction(ExtType, ValVT, MemVT) == Legal ||
- getLoadExtAction(ExtType, ValVT, MemVT) == Custom;
- }
-
- /// Same as getLoadExtAction, but for atomic loads.
- LegalizeAction getAtomicLoadExtAction(unsigned ExtType, EVT ValVT,
- EVT MemVT) const {
- if (ValVT.isExtended() || MemVT.isExtended()) return Expand;
- unsigned ValI = (unsigned)ValVT.getSimpleVT().SimpleTy;
- unsigned MemI = (unsigned)MemVT.getSimpleVT().SimpleTy;
- assert(ExtType < ISD::LAST_LOADEXT_TYPE && ValI < MVT::VALUETYPE_SIZE &&
- MemI < MVT::VALUETYPE_SIZE && "Table isn't big enough!");
- unsigned Shift = 4 * ExtType;
- LegalizeAction Action =
- (LegalizeAction)((AtomicLoadExtActions[ValI][MemI] >> Shift) & 0xf);
- assert((Action == Legal || Action == Expand) &&
- "Unsupported atomic load extension action.");
- return Action;
- }
-
- /// Return true if the specified atomic load with extension is legal on
- /// this target.
- bool isAtomicLoadExtLegal(unsigned ExtType, EVT ValVT, EVT MemVT) const {
- return getAtomicLoadExtAction(ExtType, ValVT, MemVT) == Legal;
+ bool isLoadLegalOrCustom(EVT ValVT, EVT MemVT, Align Alignment,
+ MachineMemOperand::Flags MMOFlags,
+ unsigned AddrSpace, unsigned ExtType,
+ bool Atomic) const {
+ return getLoadAction(ValVT, MemVT, Alignment, MMOFlags, AddrSpace, ExtType,
+ Atomic) == Legal ||
+ getLoadAction(ValVT, MemVT, Alignment, MMOFlags, AddrSpace, ExtType,
+ Atomic) == Custom;
}
/// Return how this store with truncation should be treated: either it is
@@ -3182,7 +3163,9 @@ class LLVM_ABI TargetLoweringBase {
LType = ISD::SEXTLOAD;
}
- return isLoadExtLegal(LType, VT, LoadVT);
+ return isLoadLegal(VT, LoadVT, Load->getAlign(),
+ getLoadMemOperandFlags(*Load, DL),
+ Load->getPointerAddressSpace(), LType, false);
}
/// Return true if any actual instruction that defines a value of type FromTy
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index bf56ffd3b4b7b..c792e6d0e0574 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -7495,12 +7495,12 @@ bool CodeGenPrepare::optimizeLoadExt(LoadInst *Load) {
uint32_t ActiveBits = DemandBits.getActiveBits();
// Avoid hoisting (and (load x) 1) since it is unlikely to be folded by the
- // target even if isLoadExtLegal says an i1 EXTLOAD is valid. For example,
- // for the AArch64 target isLoadExtLegal(ZEXTLOAD, i32, i1) returns true, but
- // (and (load x) 1) is not matched as a single instruction, rather as a LDR
- // followed by an AND.
+ // target even if isLoadLegal says an i1 EXTLOAD is valid. For example,
+ // for the AArch64 target isLoadLegal(i32, i1, ..., ZEXTLOAD, false) returns
+ // true, but (and (load x) 1) is not matched as a single instruction, rather
+ // as a LDR followed by an AND.
// TODO: Look into removing this restriction by fixing backends to either
- // return false for isLoadExtLegal for i1 or have them select this pattern to
+ // return false for isLoadLegal for i1 or have them select this pattern to
// a single instruction.
//
// Also avoid hoisting if we didn't see any ands with the exact DemandBits
@@ -7515,7 +7515,9 @@ bool CodeGenPrepare::optimizeLoadExt(LoadInst *Load) {
// Reject cases that won't be matched as extloads.
if (!LoadResultVT.bitsGT(TruncVT) || !TruncVT.isRound() ||
- !TLI->isLoadExtLegal(ISD::ZEXTLOAD, LoadResultVT, TruncVT))
+ !TLI->isLoadLegal(LoadResultVT, TruncVT, Load->getAlign(),
+ TLI->getLoadMemOperandFlags(*Load, *DL),
+ Load->getPointerAddressSpace(), ISD::ZEXTLOAD, false))
return false;
IRBuilder<> Builder(Load->getNextNode());
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 58c59628342c7..b2f36d4074cdf 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -7047,7 +7047,9 @@ bool DAGCombiner::isAndLoadExtLoad(ConstantSDNode *AndC, LoadSDNode *LoadN,
if (ExtVT == LoadedVT &&
(!LegalOperations ||
- TLI.isLoadExtLegal(ISD::ZEXTLOAD, LoadResultTy, ExtVT))) {
+ TLI.isLoadLegal(LoadResultTy, ExtVT, LoadN->getAlign(),
+ LoadN->getMemOperand()->getFlags(),
+ LoadN->getAddressSpace(), ISD::ZEXTLOAD, false))) {
// ZEXTLOAD will match without needing to change the size of the value being
// loaded.
return true;
@@ -7063,7 +7065,9 @@ bool DAGCombiner::isAndLoadExtLoad(ConstantSDNode *AndC, LoadSDNode *LoadN,
return false;
if (LegalOperations &&
- !TLI.isLoadExtLegal(ISD::ZEXTLOAD, LoadResultTy, ExtVT))
+ !TLI.isLoadLegal(LoadResultTy, ExtVT, LoadN->getAlign(),
+ LoadN->getMemOperand()->getFlags(),
+ LoadN->getAddressSpace(), ISD::ZEXTLOAD, false))
return false;
if (!TLI.shouldReduceLoadWidth(LoadN, ISD::ZEXTLOAD, ExtVT, /*ByteOffset=*/0))
@@ -7126,7 +7130,9 @@ bool DAGCombiner::isLegalNarrowLdSt(LSBaseSDNode *LDST,
return false;
if (LegalOperations &&
- !TLI.isLoadExtLegal(ExtType, Load->getValueType(0), MemVT))
+ !TLI.isLoadLegal(Load->getValueType(0), MemVT, Load->getAlign(),
+ Load->getMemOperand()->getFlags(),
+ Load->getAddressSpace(), ExtType, false))
return false;
// For the transform to be legal, the load must produce only two values
@@ -7638,7 +7644,9 @@ SDValue DAGCombiner::visitAND(SDNode *N) {
ConstantSDNode *Splat = isConstOrConstSplat(N1, true, true);
if (MLoad && MLoad->getExtensionType() == ISD::EXTLOAD && Splat) {
EVT MemVT = MLoad->getMemoryVT();
- if (TLI.isLoadExtLegal(ISD::ZEXTLOAD, VT, MemVT)) {
+ if (TLI.isLoadLegal(VT, MemVT, MLoad->getAlign(),
+ MLoad->getMemOperand()->getFlags(),
+ MLoad->getAddressSpace(), ISD::ZEXTLOAD, false)) {
// For this AND to be a zero extension of the masked load the elements
// of the BuildVec must mask the bottom bits of the extended element
// type
@@ -7785,9 +7793,10 @@ SDValue DAGCombiner::visitAND(SDNode *N) {
// If we want to change an EXTLOAD to a ZEXTLOAD, ensure a ZEXTLOAD is
// actually legal and isn't going to get expanded, else this is a false
// optimisation.
- bool CanZextLoadProfitably = TLI.isLoadExtLegal(ISD::ZEXTLOAD,
- Load->getValueType(0),
- Load->getMemoryVT());
+ bool CanZextLoadProfitably =
+ TLI.isLoadLegal(Load->getValueType(0), Load->getMemoryVT(),
+ Load->getAlign(), Load->getMemOperand()->getFlags(),
+ Load->getAddressSpace(), ISD::ZEXTLOAD, false);
// Resize the constant to the same size as the original memory access before
// extension. If it is still the AllOnesValue then this AND is completely
@@ -7979,7 +7988,9 @@ SDValue DAGCombiner::visitAND(SDNode *N) {
APInt ExtBits = APInt::getHighBitsSet(ExtBitSize, ExtBitSize - MemBitSize);
if (DAG.MaskedValueIsZero(N1, ExtBits) &&
((!LegalOperations && LN0->isSimple()) ||
- TLI.isLoadExtLegal(ISD::ZEXTLOAD, VT, MemVT))) {
+ TLI.isLoadLegal(VT, MemVT, LN0->getAlign(),
+ LN0->getMemOperand()->getFlags(),
+ LN0->getAddressSpace(), ISD::ZEXTLOAD, false))) {
SDValue ExtLoad =
DAG.getExtLoad(ISD::ZEXTLOAD, SDLoc(N0), VT, LN0->getChain(),
LN0->getBasePtr(), MemVT, LN0->getMemOperand());
@@ -9901,10 +9912,17 @@ SDValue DAGCombiner::MatchLoadCombine(SDNode *N) {
// Before legalize we can introduce too wide illegal loads which will be later
// split into legal sized loads. This enables us to combine i64 load by i8
// patterns to a couple of i32 loads on 32 bit targets.
- if (LegalOperations &&
- !TLI.isLoadExtLegal(NeedsZext ? ISD::ZEXTLOAD : ISD::NON_EXTLOAD, VT,
- MemVT))
- return SDValue();
+ if (LegalOperations) {
+ for (auto *L : Loads) {
+ if (!TLI.isLoadLegal(VT, MemVT, L->getAlign(),
+ L->getMemOperand()->getFlags(), L->getAddressSpace(),
+ NeedsZext ? ISD::ZEXTLOAD : ISD::NON_EXTLOAD,
+ false)) {
+
+ return SDValue();
+ }
+ }
+ }
// Check if the bytes of the OR we are looking at match with either big or
// little endian value load
@@ -13782,20 +13800,27 @@ SDValue DAGCombiner::visitVSELECT(SDNode *N) {
unsigned WideWidth = WideVT.getScalarSizeInBits();
bool IsSigned = isSignedIntSetCC(CC);
auto LoadExtOpcode = IsSigned ? ISD::SEXTLOAD : ISD::ZEXTLOAD;
- if (LHS.getOpcode() == ISD::LOAD && LHS.hasOneUse() &&
- SetCCWidth != 1 && SetCCWidth < WideWidth &&
- TLI.isLoadExtLegalOrCustom(LoadExtOpcode, WideVT, NarrowVT) &&
+ if (LHS.getOpcode() == ISD::LOAD && LHS.hasOneUse() && SetCCWidth != 1 &&
+ SetCCWidth < WideWidth &&
TLI.isOperationLegalOrCustom(ISD::SETCC, WideVT)) {
- // Both compare operands can be widened for free. The LHS can use an
- // extended load, and the RHS is a constant:
- // vselect (ext (setcc load(X), C)), N1, N2 -->
- // vselect (setcc extload(X), C'), N1, N2
- auto ExtOpcode = IsSigned ? ISD::SIGN_EXTEND : ISD::ZERO_EXTEND;
- SDValue WideLHS = DAG.getNode(ExtOpcode, DL, WideVT, LHS);
- SDValue WideRHS = DAG.getNode(ExtOpcode, DL, WideVT, RHS);
- EVT WideSetCCVT = getSetCCResultType(WideVT);
- SDValue WideSetCC = DAG.getSetCC(DL, WideSetCCVT, WideLHS, WideRHS, CC);
- return DAG.getSelect(DL, N1.getValueType(), WideSetCC, N1, N2);
+ LoadSDNode *Ld = cast<LoadSDNode>(LHS);
+
+ if (TLI.isLoadLegalOrCustom(WideVT, NarrowVT, Ld->getAlign(),
+ Ld->getMemOperand()->getFlags(),
+ Ld->getAddressSpace(), LoadExtOpcode,
+ false)) {
+ // Both compare operands can be widened for free. The LHS can use an
+ // extended load, and the RHS is a constant:
+ // vselect (ext (setcc load(X), C)), N1, N2 -->
+ // vselect (setcc extload(X), C'), N1, N2
+ auto ExtOpcode = IsSigned ? ISD::SIGN_EXTEND : ISD::ZERO_EXTEND;
+ SDValue WideLHS = DAG.getNode(ExtOpcode, DL, WideVT, LHS);
+ SDValue WideRHS = DAG.getNode(ExtOpcode, DL, WideVT, RHS);
+ EVT WideSetCCVT = getSetCCResultType(WideVT);
+ SDValue WideSetCC =
+ DAG.getSetCC(DL, WideSetCCVT, WideLHS, WideRHS, CC);
+ return DAG.getSelect(DL, N1.getValueType(), WideSetCC, N1, N2);
+ }
}
}
@@ -14230,8 +14255,12 @@ static SDValue tryToFoldExtendSelectLoad(SDNode *N, const TargetLowering &TLI,
// Combine2), so we should conservatively check the OperationAction.
LoadSDNode *Load1 = cast<LoadSDNode>(Op1);
LoadSDNode *Load2 = cast<LoadSDNode>(Op2);
- if (!TLI.isLoadExtLegal(ExtLoadOpcode, VT, Load1->getMemoryVT()) ||
- !TLI.isLoadExtLegal(ExtLoadOpcode, VT, Load2->getMemoryVT()) ||
+ if (!TLI.isLoadLegal(VT, Load1->getMemoryVT(), Load1->getAlign(),
+ Load1->getMemOperand()->getFlags(),
+ Load1->getAddressSpace(), ExtLoadOpcode, false) ||
+ !TLI.isLoadLegal(VT, Load2->getMemoryVT(), Load2->getAlign(),
+ Load2->getMemOperand()->getFlags(),
+ Load2->getAddressSpace(), ExtLoadOpcode, false) ||
(N0->getOpcode() == ISD::VSELECT && Level >= AfterLegalizeTypes &&
TLI.getOperationAction(ISD::VSELECT, VT) != TargetLowering::Legal))
return SDValue();
@@ -14455,13 +14484,17 @@ SDValue DAGCombiner::CombineExtLoad(SDNode *N) {
// Try to split the vector types to get down to legal types.
EVT SplitSrcVT = SrcVT;
EVT SplitDstVT = DstVT;
- while (!TLI.isLoadExtLegalOrCustom(ExtType, SplitDstVT, SplitSrcVT) &&
+ while (!TLI.isLoadLegalOrCustom(SplitDstVT, SplitSrcVT, LN0->getAlign(),
+ LN0->getMemOperand()->getFlags(),
+ LN0->getAddressSpace(), ExtType, false) &&
SplitSrcVT.getVectorNumElements() > 1) {
SplitDstVT = DAG.GetSplitDestVTs(SplitDstVT).first;
SplitSrcVT = DAG.GetSplitDestVTs(SplitSrcVT).first;
}
- if (!TLI.isLoadExtLegalOrCustom(ExtType, SplitDstVT, SplitSrcVT))
+ if (!TLI.isLoadLegalOrCustom(SplitDstVT, SplitSrcVT, LN0->getAlign(),
+ LN0->getMemOperand()->getFlags(),
+ LN0->getAddressSpace(), ExtType, false))
return SDValue();
assert(!DstVT.isScalableVector() && "Unexpected scalable vector type");
@@ -14534,7 +14567,9 @@ SDValue DAGCombiner::CombineZExtLogicopShiftLoad(SDNode *N) {
return SDValue();
LoadSDNode *Load = cast<LoadSDNode>(N1.getOperand(0));
EVT MemVT = Load->getMemoryVT();
- if (!TLI.isLoadExtLegal(ISD::ZEXTLOAD, VT, MemVT) ||
+ if (!TLI.isLoadLegal(VT, MemVT, Load->getAlign(),
+ Load->getMemOperand()->getFlags(),
+ Load->getAddressSpace(), ISD::ZEXTLOAD, false) ||
Load->getExtensionType() == ISD::SEXTLOAD || Load->isIndexed())
return SDValue();
@@ -14647,7 +14682,9 @@ static SDValue tryToFoldExtOfExtload(SelectionDAG &DAG, DAGCombiner &Combiner,
EVT MemVT = OldExtLoad->getMemoryVT();
if ((LegalOperations || !OldExtLoad->isSimple() || VT.isVector()) &&
- !TLI.isLoadExtLegal(ExtLoadType, VT, MemVT))
+ !TLI.isLoadLegal(VT, MemVT, OldExtLoad->getAlign(),
+ OldExtLoad->getMemOperand()->getFlags(),
+ OldExtLoad->getAddressSpace(), ExtLoadType, false))
return SDValue();
SDLoc DL(OldExtLoad);
@@ -14707,7 +14744,9 @@ static SDValue tryToFoldExtOfLoad(SelectionDAG &DAG, DAGCombiner &Combiner,
// code generation being the result of that target's implementation of
// isVectorLoadExtDesirable().
if ((LegalOperations || VT.isFixedLengthVector() || !Load->isSimple()) &&
- !TLI.isLoadExtLegal(ExtLoadType, VT, Load->getValueType(0)))
+ !TLI.isLoadLegal(VT, Load->getValueType(0), Load->getAlign(),
+ Load->getMemOperand()->getFlags(),
+ ...
[truncated]
|
Member
|
@llvm/pr-subscribers-llvm-selectiondag Author: Demetrius Kanios (QuantumSegfault) ChangesAlternative approach to the same goals as #162407 This takes This is fully backwards compatible, with the existing setLoadExtAction working as before. But this allows targets to override the query to make more use of the information. Patch is 45.07 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/181104.diff 10 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/BasicTTIImpl.h b/llvm/include/llvm/CodeGen/BasicTTIImpl.h
index 21afcbefdf719..132dc9d18845d 100644
--- a/llvm/include/llvm/CodeGen/BasicTTIImpl.h
+++ b/llvm/include/llvm/CodeGen/BasicTTIImpl.h
@@ -28,6 +28,7 @@
#include "llvm/Analysis/TargetTransformInfoImpl.h"
#include "llvm/Analysis/ValueTracking.h"
#include "llvm/CodeGen/ISDOpcodes.h"
+#include "llvm/CodeGen/MachineMemOperand.h"
#include "llvm/CodeGen/TargetLowering.h"
#include "llvm/CodeGen/TargetSubtargetInfo.h"
#include "llvm/CodeGen/ValueTypes.h"
@@ -1261,10 +1262,41 @@ class BasicTTIImplBase : public TargetTransformInfoImplCRTPBase<T> {
EVT ExtVT = EVT::getEVT(Dst);
EVT LoadVT = EVT::getEVT(Src);
unsigned LType =
- ((Opcode == Instruction::ZExt) ? ISD::ZEXTLOAD : ISD::SEXTLOAD);
- if (DstLT.first == SrcLT.first &&
- TLI->isLoadExtLegal(LType, ExtVT, LoadVT))
- return 0;
+ ((Opcode == Instruction::ZExt) ? ISD::ZEXTLOAD : ISD::SEXTLOAD);
+ if (I) {
+ if (auto *LI = dyn_cast<LoadInst>(I->getOperand(0))) {
+ if (DstLT.first == SrcLT.first &&
+ TLI->isLoadLegal(
+ ExtVT, LoadVT, LI->getAlign(),
+ TLI->getLoadMemOperandFlags(*LI, DL),
+ LI->getPointerAddressSpace(), LType, false))
+ return 0;
+ } else if (auto *II = dyn_cast<IntrinsicInst>(I->getOperand(0))) {
+ switch (II->getIntrinsicID()) {
+ case Intrinsic::masked_load: {
+ Type *PtrType = II->getArgOperand(0)->getType();
+ assert(PtrType->isPointerTy());
+
+ auto MMOFlags = MachineMemOperand::MOLoad;
+ if (I->hasMetadata(LLVMContext::MD_nontemporal))
+ MMOFlags |= MachineMemOperand::MONonTemporal;
+ if (I->hasMetadata(LLVMContext::MD_invariant_load))
+ MMOFlags |= MachineMemOperand::MOInvariant;
+
+ if (DstLT.first == SrcLT.first &&
+ TLI->isLoadLegal(ExtVT, LoadVT,
+ II->getParamAlign(0).valueOrOne(), MMOFlags,
+ PtrType->getPointerAddressSpace(), LType,
+ false))
+ return 0;
+
+ break;
+ }
+ default:
+ break;
+ }
+ }
+ }
}
break;
case Instruction::AddrSpaceCast:
@@ -1556,7 +1588,9 @@ class BasicTTIImplBase : public TargetTransformInfoImplCRTPBase<T> {
if (Opcode == Instruction::Store)
LA = getTLI()->getTruncStoreAction(LT.second, MemVT);
else
- LA = getTLI()->getLoadExtAction(ISD::EXTLOAD, LT.second, MemVT);
+ LA = getTLI()->getLoadAction(LT.second, MemVT, Alignment,
+ MachineMemOperand::Flags::MOLoad,
+ AddressSpace, ISD::EXTLOAD, false);
if (LA != TargetLowering::Legal && LA != TargetLowering::Custom) {
// This is a vector load/store for some illegal type that is scalarized.
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index ada4ffd3bcc89..1bdf87e92edcc 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -31,6 +31,7 @@
#include "llvm/CodeGen/ISDOpcodes.h"
#include "llvm/CodeGen/LibcallLoweringInfo.h"
#include "llvm/CodeGen/LowLevelTypeUtils.h"
+#include "llvm/CodeGen/MachineMemOperand.h"
#include "llvm/CodeGen/MachineRegisterInfo.h"
#include "llvm/CodeGen/RuntimeLibcallUtil.h"
#include "llvm/CodeGen/SelectionDAG.h"
@@ -1511,49 +1512,29 @@ class LLVM_ABI TargetLoweringBase {
/// Return how this load with extension should be treated: either it is legal,
/// needs to be promoted to a larger size, needs to be expanded to some other
/// code sequence, or the target has a custom expander for it.
- LegalizeAction getLoadExtAction(unsigned ExtType, EVT ValVT,
- EVT MemVT) const {
- if (ValVT.isExtended() || MemVT.isExtended()) return Expand;
- unsigned ValI = (unsigned) ValVT.getSimpleVT().SimpleTy;
- unsigned MemI = (unsigned) MemVT.getSimpleVT().SimpleTy;
- assert(ExtType < ISD::LAST_LOADEXT_TYPE && ValI < MVT::VALUETYPE_SIZE &&
- MemI < MVT::VALUETYPE_SIZE && "Table isn't big enough!");
- unsigned Shift = 4 * ExtType;
- return (LegalizeAction)((LoadExtActions[ValI][MemI] >> Shift) & 0xf);
- }
+ virtual LegalizeAction getLoadAction(EVT ValVT, EVT MemVT, Align Alignment,
+ MachineMemOperand::Flags MMOFlags,
+ unsigned AddrSpace, unsigned ExtType,
+ bool Atomic) const;
/// Return true if the specified load with extension is legal on this target.
- bool isLoadExtLegal(unsigned ExtType, EVT ValVT, EVT MemVT) const {
- return getLoadExtAction(ExtType, ValVT, MemVT) == Legal;
+ bool isLoadLegal(EVT ValVT, EVT MemVT, Align Alignment,
+ MachineMemOperand::Flags MMOFlags, unsigned AddrSpace,
+ unsigned ExtType, bool Atomic) const {
+ return getLoadAction(ValVT, MemVT, Alignment, MMOFlags, AddrSpace, ExtType,
+ Atomic) == Legal;
}
/// Return true if the specified load with extension is legal or custom
/// on this target.
- bool isLoadExtLegalOrCustom(unsigned ExtType, EVT ValVT, EVT MemVT) const {
- return getLoadExtAction(ExtType, ValVT, MemVT) == Legal ||
- getLoadExtAction(ExtType, ValVT, MemVT) == Custom;
- }
-
- /// Same as getLoadExtAction, but for atomic loads.
- LegalizeAction getAtomicLoadExtAction(unsigned ExtType, EVT ValVT,
- EVT MemVT) const {
- if (ValVT.isExtended() || MemVT.isExtended()) return Expand;
- unsigned ValI = (unsigned)ValVT.getSimpleVT().SimpleTy;
- unsigned MemI = (unsigned)MemVT.getSimpleVT().SimpleTy;
- assert(ExtType < ISD::LAST_LOADEXT_TYPE && ValI < MVT::VALUETYPE_SIZE &&
- MemI < MVT::VALUETYPE_SIZE && "Table isn't big enough!");
- unsigned Shift = 4 * ExtType;
- LegalizeAction Action =
- (LegalizeAction)((AtomicLoadExtActions[ValI][MemI] >> Shift) & 0xf);
- assert((Action == Legal || Action == Expand) &&
- "Unsupported atomic load extension action.");
- return Action;
- }
-
- /// Return true if the specified atomic load with extension is legal on
- /// this target.
- bool isAtomicLoadExtLegal(unsigned ExtType, EVT ValVT, EVT MemVT) const {
- return getAtomicLoadExtAction(ExtType, ValVT, MemVT) == Legal;
+ bool isLoadLegalOrCustom(EVT ValVT, EVT MemVT, Align Alignment,
+ MachineMemOperand::Flags MMOFlags,
+ unsigned AddrSpace, unsigned ExtType,
+ bool Atomic) const {
+ return getLoadAction(ValVT, MemVT, Alignment, MMOFlags, AddrSpace, ExtType,
+ Atomic) == Legal ||
+ getLoadAction(ValVT, MemVT, Alignment, MMOFlags, AddrSpace, ExtType,
+ Atomic) == Custom;
}
/// Return how this store with truncation should be treated: either it is
@@ -3182,7 +3163,9 @@ class LLVM_ABI TargetLoweringBase {
LType = ISD::SEXTLOAD;
}
- return isLoadExtLegal(LType, VT, LoadVT);
+ return isLoadLegal(VT, LoadVT, Load->getAlign(),
+ getLoadMemOperandFlags(*Load, DL),
+ Load->getPointerAddressSpace(), LType, false);
}
/// Return true if any actual instruction that defines a value of type FromTy
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index bf56ffd3b4b7b..c792e6d0e0574 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -7495,12 +7495,12 @@ bool CodeGenPrepare::optimizeLoadExt(LoadInst *Load) {
uint32_t ActiveBits = DemandBits.getActiveBits();
// Avoid hoisting (and (load x) 1) since it is unlikely to be folded by the
- // target even if isLoadExtLegal says an i1 EXTLOAD is valid. For example,
- // for the AArch64 target isLoadExtLegal(ZEXTLOAD, i32, i1) returns true, but
- // (and (load x) 1) is not matched as a single instruction, rather as a LDR
- // followed by an AND.
+ // target even if isLoadLegal says an i1 EXTLOAD is valid. For example,
+ // for the AArch64 target isLoadLegal(i32, i1, ..., ZEXTLOAD, false) returns
+ // true, but (and (load x) 1) is not matched as a single instruction, rather
+ // as a LDR followed by an AND.
// TODO: Look into removing this restriction by fixing backends to either
- // return false for isLoadExtLegal for i1 or have them select this pattern to
+ // return false for isLoadLegal for i1 or have them select this pattern to
// a single instruction.
//
// Also avoid hoisting if we didn't see any ands with the exact DemandBits
@@ -7515,7 +7515,9 @@ bool CodeGenPrepare::optimizeLoadExt(LoadInst *Load) {
// Reject cases that won't be matched as extloads.
if (!LoadResultVT.bitsGT(TruncVT) || !TruncVT.isRound() ||
- !TLI->isLoadExtLegal(ISD::ZEXTLOAD, LoadResultVT, TruncVT))
+ !TLI->isLoadLegal(LoadResultVT, TruncVT, Load->getAlign(),
+ TLI->getLoadMemOperandFlags(*Load, *DL),
+ Load->getPointerAddressSpace(), ISD::ZEXTLOAD, false))
return false;
IRBuilder<> Builder(Load->getNextNode());
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 58c59628342c7..b2f36d4074cdf 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -7047,7 +7047,9 @@ bool DAGCombiner::isAndLoadExtLoad(ConstantSDNode *AndC, LoadSDNode *LoadN,
if (ExtVT == LoadedVT &&
(!LegalOperations ||
- TLI.isLoadExtLegal(ISD::ZEXTLOAD, LoadResultTy, ExtVT))) {
+ TLI.isLoadLegal(LoadResultTy, ExtVT, LoadN->getAlign(),
+ LoadN->getMemOperand()->getFlags(),
+ LoadN->getAddressSpace(), ISD::ZEXTLOAD, false))) {
// ZEXTLOAD will match without needing to change the size of the value being
// loaded.
return true;
@@ -7063,7 +7065,9 @@ bool DAGCombiner::isAndLoadExtLoad(ConstantSDNode *AndC, LoadSDNode *LoadN,
return false;
if (LegalOperations &&
- !TLI.isLoadExtLegal(ISD::ZEXTLOAD, LoadResultTy, ExtVT))
+ !TLI.isLoadLegal(LoadResultTy, ExtVT, LoadN->getAlign(),
+ LoadN->getMemOperand()->getFlags(),
+ LoadN->getAddressSpace(), ISD::ZEXTLOAD, false))
return false;
if (!TLI.shouldReduceLoadWidth(LoadN, ISD::ZEXTLOAD, ExtVT, /*ByteOffset=*/0))
@@ -7126,7 +7130,9 @@ bool DAGCombiner::isLegalNarrowLdSt(LSBaseSDNode *LDST,
return false;
if (LegalOperations &&
- !TLI.isLoadExtLegal(ExtType, Load->getValueType(0), MemVT))
+ !TLI.isLoadLegal(Load->getValueType(0), MemVT, Load->getAlign(),
+ Load->getMemOperand()->getFlags(),
+ Load->getAddressSpace(), ExtType, false))
return false;
// For the transform to be legal, the load must produce only two values
@@ -7638,7 +7644,9 @@ SDValue DAGCombiner::visitAND(SDNode *N) {
ConstantSDNode *Splat = isConstOrConstSplat(N1, true, true);
if (MLoad && MLoad->getExtensionType() == ISD::EXTLOAD && Splat) {
EVT MemVT = MLoad->getMemoryVT();
- if (TLI.isLoadExtLegal(ISD::ZEXTLOAD, VT, MemVT)) {
+ if (TLI.isLoadLegal(VT, MemVT, MLoad->getAlign(),
+ MLoad->getMemOperand()->getFlags(),
+ MLoad->getAddressSpace(), ISD::ZEXTLOAD, false)) {
// For this AND to be a zero extension of the masked load the elements
// of the BuildVec must mask the bottom bits of the extended element
// type
@@ -7785,9 +7793,10 @@ SDValue DAGCombiner::visitAND(SDNode *N) {
// If we want to change an EXTLOAD to a ZEXTLOAD, ensure a ZEXTLOAD is
// actually legal and isn't going to get expanded, else this is a false
// optimisation.
- bool CanZextLoadProfitably = TLI.isLoadExtLegal(ISD::ZEXTLOAD,
- Load->getValueType(0),
- Load->getMemoryVT());
+ bool CanZextLoadProfitably =
+ TLI.isLoadLegal(Load->getValueType(0), Load->getMemoryVT(),
+ Load->getAlign(), Load->getMemOperand()->getFlags(),
+ Load->getAddressSpace(), ISD::ZEXTLOAD, false);
// Resize the constant to the same size as the original memory access before
// extension. If it is still the AllOnesValue then this AND is completely
@@ -7979,7 +7988,9 @@ SDValue DAGCombiner::visitAND(SDNode *N) {
APInt ExtBits = APInt::getHighBitsSet(ExtBitSize, ExtBitSize - MemBitSize);
if (DAG.MaskedValueIsZero(N1, ExtBits) &&
((!LegalOperations && LN0->isSimple()) ||
- TLI.isLoadExtLegal(ISD::ZEXTLOAD, VT, MemVT))) {
+ TLI.isLoadLegal(VT, MemVT, LN0->getAlign(),
+ LN0->getMemOperand()->getFlags(),
+ LN0->getAddressSpace(), ISD::ZEXTLOAD, false))) {
SDValue ExtLoad =
DAG.getExtLoad(ISD::ZEXTLOAD, SDLoc(N0), VT, LN0->getChain(),
LN0->getBasePtr(), MemVT, LN0->getMemOperand());
@@ -9901,10 +9912,17 @@ SDValue DAGCombiner::MatchLoadCombine(SDNode *N) {
// Before legalize we can introduce too wide illegal loads which will be later
// split into legal sized loads. This enables us to combine i64 load by i8
// patterns to a couple of i32 loads on 32 bit targets.
- if (LegalOperations &&
- !TLI.isLoadExtLegal(NeedsZext ? ISD::ZEXTLOAD : ISD::NON_EXTLOAD, VT,
- MemVT))
- return SDValue();
+ if (LegalOperations) {
+ for (auto *L : Loads) {
+ if (!TLI.isLoadLegal(VT, MemVT, L->getAlign(),
+ L->getMemOperand()->getFlags(), L->getAddressSpace(),
+ NeedsZext ? ISD::ZEXTLOAD : ISD::NON_EXTLOAD,
+ false)) {
+
+ return SDValue();
+ }
+ }
+ }
// Check if the bytes of the OR we are looking at match with either big or
// little endian value load
@@ -13782,20 +13800,27 @@ SDValue DAGCombiner::visitVSELECT(SDNode *N) {
unsigned WideWidth = WideVT.getScalarSizeInBits();
bool IsSigned = isSignedIntSetCC(CC);
auto LoadExtOpcode = IsSigned ? ISD::SEXTLOAD : ISD::ZEXTLOAD;
- if (LHS.getOpcode() == ISD::LOAD && LHS.hasOneUse() &&
- SetCCWidth != 1 && SetCCWidth < WideWidth &&
- TLI.isLoadExtLegalOrCustom(LoadExtOpcode, WideVT, NarrowVT) &&
+ if (LHS.getOpcode() == ISD::LOAD && LHS.hasOneUse() && SetCCWidth != 1 &&
+ SetCCWidth < WideWidth &&
TLI.isOperationLegalOrCustom(ISD::SETCC, WideVT)) {
- // Both compare operands can be widened for free. The LHS can use an
- // extended load, and the RHS is a constant:
- // vselect (ext (setcc load(X), C)), N1, N2 -->
- // vselect (setcc extload(X), C'), N1, N2
- auto ExtOpcode = IsSigned ? ISD::SIGN_EXTEND : ISD::ZERO_EXTEND;
- SDValue WideLHS = DAG.getNode(ExtOpcode, DL, WideVT, LHS);
- SDValue WideRHS = DAG.getNode(ExtOpcode, DL, WideVT, RHS);
- EVT WideSetCCVT = getSetCCResultType(WideVT);
- SDValue WideSetCC = DAG.getSetCC(DL, WideSetCCVT, WideLHS, WideRHS, CC);
- return DAG.getSelect(DL, N1.getValueType(), WideSetCC, N1, N2);
+ LoadSDNode *Ld = cast<LoadSDNode>(LHS);
+
+ if (TLI.isLoadLegalOrCustom(WideVT, NarrowVT, Ld->getAlign(),
+ Ld->getMemOperand()->getFlags(),
+ Ld->getAddressSpace(), LoadExtOpcode,
+ false)) {
+ // Both compare operands can be widened for free. The LHS can use an
+ // extended load, and the RHS is a constant:
+ // vselect (ext (setcc load(X), C)), N1, N2 -->
+ // vselect (setcc extload(X), C'), N1, N2
+ auto ExtOpcode = IsSigned ? ISD::SIGN_EXTEND : ISD::ZERO_EXTEND;
+ SDValue WideLHS = DAG.getNode(ExtOpcode, DL, WideVT, LHS);
+ SDValue WideRHS = DAG.getNode(ExtOpcode, DL, WideVT, RHS);
+ EVT WideSetCCVT = getSetCCResultType(WideVT);
+ SDValue WideSetCC =
+ DAG.getSetCC(DL, WideSetCCVT, WideLHS, WideRHS, CC);
+ return DAG.getSelect(DL, N1.getValueType(), WideSetCC, N1, N2);
+ }
}
}
@@ -14230,8 +14255,12 @@ static SDValue tryToFoldExtendSelectLoad(SDNode *N, const TargetLowering &TLI,
// Combine2), so we should conservatively check the OperationAction.
LoadSDNode *Load1 = cast<LoadSDNode>(Op1);
LoadSDNode *Load2 = cast<LoadSDNode>(Op2);
- if (!TLI.isLoadExtLegal(ExtLoadOpcode, VT, Load1->getMemoryVT()) ||
- !TLI.isLoadExtLegal(ExtLoadOpcode, VT, Load2->getMemoryVT()) ||
+ if (!TLI.isLoadLegal(VT, Load1->getMemoryVT(), Load1->getAlign(),
+ Load1->getMemOperand()->getFlags(),
+ Load1->getAddressSpace(), ExtLoadOpcode, false) ||
+ !TLI.isLoadLegal(VT, Load2->getMemoryVT(), Load2->getAlign(),
+ Load2->getMemOperand()->getFlags(),
+ Load2->getAddressSpace(), ExtLoadOpcode, false) ||
(N0->getOpcode() == ISD::VSELECT && Level >= AfterLegalizeTypes &&
TLI.getOperationAction(ISD::VSELECT, VT) != TargetLowering::Legal))
return SDValue();
@@ -14455,13 +14484,17 @@ SDValue DAGCombiner::CombineExtLoad(SDNode *N) {
// Try to split the vector types to get down to legal types.
EVT SplitSrcVT = SrcVT;
EVT SplitDstVT = DstVT;
- while (!TLI.isLoadExtLegalOrCustom(ExtType, SplitDstVT, SplitSrcVT) &&
+ while (!TLI.isLoadLegalOrCustom(SplitDstVT, SplitSrcVT, LN0->getAlign(),
+ LN0->getMemOperand()->getFlags(),
+ LN0->getAddressSpace(), ExtType, false) &&
SplitSrcVT.getVectorNumElements() > 1) {
SplitDstVT = DAG.GetSplitDestVTs(SplitDstVT).first;
SplitSrcVT = DAG.GetSplitDestVTs(SplitSrcVT).first;
}
- if (!TLI.isLoadExtLegalOrCustom(ExtType, SplitDstVT, SplitSrcVT))
+ if (!TLI.isLoadLegalOrCustom(SplitDstVT, SplitSrcVT, LN0->getAlign(),
+ LN0->getMemOperand()->getFlags(),
+ LN0->getAddressSpace(), ExtType, false))
return SDValue();
assert(!DstVT.isScalableVector() && "Unexpected scalable vector type");
@@ -14534,7 +14567,9 @@ SDValue DAGCombiner::CombineZExtLogicopShiftLoad(SDNode *N) {
return SDValue();
LoadSDNode *Load = cast<LoadSDNode>(N1.getOperand(0));
EVT MemVT = Load->getMemoryVT();
- if (!TLI.isLoadExtLegal(ISD::ZEXTLOAD, VT, MemVT) ||
+ if (!TLI.isLoadLegal(VT, MemVT, Load->getAlign(),
+ Load->getMemOperand()->getFlags(),
+ Load->getAddressSpace(), ISD::ZEXTLOAD, false) ||
Load->getExtensionType() == ISD::SEXTLOAD || Load->isIndexed())
return SDValue();
@@ -14647,7 +14682,9 @@ static SDValue tryToFoldExtOfExtload(SelectionDAG &DAG, DAGCombiner &Combiner,
EVT MemVT = OldExtLoad->getMemoryVT();
if ((LegalOperations || !OldExtLoad->isSimple() || VT.isVector()) &&
- !TLI.isLoadExtLegal(ExtLoadType, VT, MemVT))
+ !TLI.isLoadLegal(VT, MemVT, OldExtLoad->getAlign(),
+ OldExtLoad->getMemOperand()->getFlags(),
+ OldExtLoad->getAddressSpace(), ExtLoadType, false))
return SDValue();
SDLoc DL(OldExtLoad);
@@ -14707,7 +14744,9 @@ static SDValue tryToFoldExtOfLoad(SelectionDAG &DAG, DAGCombiner &Combiner,
// code generation being the result of that target's implementation of
// isVectorLoadExtDesirable().
if ((LegalOperations || VT.isFixedLengthVector() || !Load->isSimple()) &&
- !TLI.isLoadExtLegal(ExtLoadType, VT, Load->getValueType(0)))
+ !TLI.isLoadLegal(VT, Load->getValueType(0), Load->getAlign(),
+ Load->getMemOperand()->getFlags(),
+ ...
[truncated]
|
Contributor
Author
|
Requesting review |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Contributor
Author
|
One thing I don't quite understand is the relationship between The default for It's like there's two layers? A very coarse check of whether a particular ValTy can be loaded into, then the more detailed getLoadExtAction for information about the actual legality of specific memory ops (i.e. MemTy loading from and ext-type). But it seems some DAGCombiners check getLoadExtAction without verifying with Anybody have any insight? I'm specifically looking at the I guess the main thing that matters is whether this change is enough, or a similar expansion of input needs to happen for what is currently just These changes should suffice for my purposes in the WASM backend. We can also make the change more obvious/breaking by removing the tables and |
Contributor
Author
arsenm
reviewed
Feb 25, 2026
Contributor
arsenm
left a comment
arsenm
left a comment
There was a problem hiding this comment.
Will also need the mirror store case, but best for another PR
| bool isLoadLegal(EVT ValVT, EVT MemVT, Align Alignment, | ||
| MachineMemOperand::Flags MMOFlags, unsigned AddrSpace, | ||
| unsigned ExtType, bool Atomic) const { | ||
| return getLoadAction(ValVT, MemVT, Alignment, MMOFlags, AddrSpace, ExtType, |
Contributor
There was a problem hiding this comment.
Can this early exit if the load is reported as legal from the existing tables, before falling back to the virtual getLoadActionImpl?
Contributor
Author
There was a problem hiding this comment.
There is no getLoadActionImpl ATM. I've set getLoadAction itself as virtual so targets can do whatever they wish with the default implementation. Call it as the default at the end. Short-circuit the custom logic if the builtin tables yield some particular value. Not use the tables at all.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Alternative approach to the same goals as #162407
This takes
TargetLoweringBase::getLoadExtAction, makes it virtual, mergesgetAtomicLoadExtActioninto it, and adds more inputs for relavent information (alignment,, address space).This is fully backwards compatible, with the existing setLoadExtAction working as before. But this allows targets to override the query to make more use of the information.