Convert LoadExtActions to a map #157627
Draft
Convert LoadExtActions to a map #157627
Conversation
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
RKSimon
requested changes
Sep 9, 2025
|
@llvm/pr-subscribers-llvm-selectiondag Author: Sam Parker (sparker-arm) ChangesSo we can store actions per address space. Patch is 57.54 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/157627.diff 2 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index 2ba8b29e775e0..d295cfa043503 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -200,11 +200,11 @@ class LLVM_ABI TargetLoweringBase {
/// This enum indicates whether operations are valid for a target, and if not,
/// what action should be used to make them valid.
enum LegalizeAction : uint8_t {
- Legal, // The target natively supports this operation.
- Promote, // This operation should be executed in a larger type.
- Expand, // Try to expand this to other ops, otherwise use a libcall.
- LibCall, // Don't try to expand this to other ops, always use a libcall.
- Custom // Use the LowerOperation hook to implement custom lowering.
+ Legal, // The target natively supports this operation.
+ Promote, // This operation should be executed in a larger type.
+ Expand, // Try to expand this to other ops, otherwise use a libcall.
+ LibCall, // Don't try to expand this to other ops, always use a libcall.
+ Custom // Use the LowerOperation hook to implement custom lowering.
};
/// This enum indicates whether a types are legal for a target, and if not,
@@ -220,13 +220,13 @@ class LLVM_ABI TargetLoweringBase {
TypeWidenVector, // This vector should be widened into a larger vector.
TypePromoteFloat, // Replace this float with a larger one.
TypeSoftPromoteHalf, // Soften half to i16 and use float to do arithmetic.
- TypeScalarizeScalableVector, // This action is explicitly left unimplemented.
- // While it is theoretically possible to
- // legalize operations on scalable types with a
- // loop that handles the vscale * #lanes of the
- // vector, this is non-trivial at SelectionDAG
- // level and these types are better to be
- // widened or promoted.
+ TypeScalarizeScalableVector, // This action is explicitly left
+ // unimplemented. While it is theoretically
+ // possible to legalize operations on scalable
+ // types with a loop that handles the vscale *
+ // #lanes of the vector, this is non-trivial at
+ // SelectionDAG level and these types are
+ // better to be widened or promoted.
};
/// LegalizeKind holds the legalization kind that needs to happen to EVT
@@ -235,18 +235,18 @@ class LLVM_ABI TargetLoweringBase {
/// Enum that describes how the target represents true/false values.
enum BooleanContent {
- UndefinedBooleanContent, // Only bit 0 counts, the rest can hold garbage.
- ZeroOrOneBooleanContent, // All bits zero except for bit 0.
+ UndefinedBooleanContent, // Only bit 0 counts, the rest can hold garbage.
+ ZeroOrOneBooleanContent, // All bits zero except for bit 0.
ZeroOrNegativeOneBooleanContent // All bits equal to bit 0.
};
/// Enum that describes what type of support for selects the target has.
enum SelectSupportKind {
- ScalarValSelect, // The target supports scalar selects (ex: cmov).
- ScalarCondVectorVal, // The target supports selects with a scalar condition
- // and vector values (ex: cmov).
- VectorMaskSelect // The target supports vector selects with a vector
- // mask (ex: x86 blends).
+ ScalarValSelect, // The target supports scalar selects (ex: cmov).
+ ScalarCondVectorVal, // The target supports selects with a scalar condition
+ // and vector values (ex: cmov).
+ VectorMaskSelect // The target supports vector selects with a vector
+ // mask (ex: x86 blends).
};
/// Enum that specifies what an atomic load/AtomicRMWInst is expanded
@@ -284,9 +284,9 @@ class LLVM_ABI TargetLoweringBase {
/// Enum that specifies when a float negation is beneficial.
enum class NegatibleCost {
- Cheaper = 0, // Negated expression is cheaper.
- Neutral = 1, // Negated expression has the same cost.
- Expensive = 2 // Negated expression is more expensive.
+ Cheaper = 0, // Negated expression is cheaper.
+ Neutral = 1, // Negated expression has the same cost.
+ Expensive = 2 // Negated expression is more expensive.
};
/// Enum of different potentially desirable ways to fold (and/or (setcc ...),
@@ -361,9 +361,7 @@ class LLVM_ABI TargetLoweringBase {
virtual ~TargetLoweringBase();
/// Return true if the target support strict float operation
- bool isStrictFPEnabled() const {
- return IsStrictFPEnabled;
- }
+ bool isStrictFPEnabled() const { return IsStrictFPEnabled; }
protected:
/// Initialize all of the actions to default values.
@@ -454,7 +452,8 @@ class LLVM_ABI TargetLoweringBase {
/// This callback is used to inspect load/store instructions and add
/// target-specific MachineMemOperand flags to them. The default
/// implementation does nothing.
- virtual MachineMemOperand::Flags getTargetMMOFlags(const Instruction &I) const {
+ virtual MachineMemOperand::Flags
+ getTargetMMOFlags(const Instruction &I) const {
return MachineMemOperand::MONone;
}
@@ -525,9 +524,7 @@ class LLVM_ABI TargetLoweringBase {
/// a constant pool load whose address depends on the select condition. The
/// parameter may be used to differentiate a select with FP compare from
/// integer compare.
- virtual bool reduceSelectOfFPConstantLoads(EVT CmpOpVT) const {
- return true;
- }
+ virtual bool reduceSelectOfFPConstantLoads(EVT CmpOpVT) const { return true; }
/// Does the target have multiple (allocatable) condition registers that
/// can be used to store the results of comparisons for use by selects
@@ -586,9 +583,7 @@ class LLVM_ABI TargetLoweringBase {
virtual bool isIntDivCheap(EVT VT, AttributeList Attr) const { return false; }
/// Return true if the target can handle a standalone remainder operation.
- virtual bool hasStandaloneRem(EVT VT) const {
- return true;
- }
+ virtual bool hasStandaloneRem(EVT VT) const { return true; }
/// Return true if SQRT(X) shouldn't be replaced with X*RSQRT(X).
virtual bool isFsqrtCheap(SDValue X, SelectionDAG &DAG) const {
@@ -597,11 +592,7 @@ class LLVM_ABI TargetLoweringBase {
}
/// Reciprocal estimate status values used by the functions below.
- enum ReciprocalEstimate : int {
- Unspecified = -1,
- Disabled = 0,
- Enabled = 1
- };
+ enum ReciprocalEstimate : int { Unspecified = -1, Disabled = 0, Enabled = 1 };
/// Return a ReciprocalEstimate enum value for a square root of the given type
/// based on the function's attributes. If the operation is not overridden by
@@ -720,9 +711,7 @@ class LLVM_ABI TargetLoweringBase {
/// Allow store merging for the specified type after legalization in addition
/// to before legalization. This may transform stores that do not exist
/// earlier (for example, stores created from intrinsics).
- virtual bool mergeStoresAfterLegalization(EVT MemVT) const {
- return true;
- }
+ virtual bool mergeStoresAfterLegalization(EVT MemVT) const { return true; }
/// Returns if it's reasonable to merge stores to MemVT size.
virtual bool canMergeStoresTo(unsigned AS, EVT MemVT,
@@ -731,19 +720,13 @@ class LLVM_ABI TargetLoweringBase {
}
/// Return true if it is cheap to speculate a call to intrinsic cttz.
- virtual bool isCheapToSpeculateCttz(Type *Ty) const {
- return false;
- }
+ virtual bool isCheapToSpeculateCttz(Type *Ty) const { return false; }
/// Return true if it is cheap to speculate a call to intrinsic ctlz.
- virtual bool isCheapToSpeculateCtlz(Type *Ty) const {
- return false;
- }
+ virtual bool isCheapToSpeculateCtlz(Type *Ty) const { return false; }
/// Return true if ctlz instruction is fast.
- virtual bool isCtlzFast() const {
- return false;
- }
+ virtual bool isCtlzFast() const { return false; }
/// Return true if ctpop instruction is fast.
virtual bool isCtpopFast(EVT VT) const {
@@ -796,9 +779,7 @@ class LLVM_ABI TargetLoweringBase {
/// This should be true when it takes more than one instruction to lower
/// setcc (cmp+set on x86 scalar), when bitwise ops are faster than logic on
/// condition bits (crand on PowerPC), and/or when reducing cmp+br is a win.
- virtual bool convertSetCCLogicToBitwiseLogic(EVT VT) const {
- return false;
- }
+ virtual bool convertSetCCLogicToBitwiseLogic(EVT VT) const { return false; }
/// Return the preferred operand type if the target has a quick way to compare
/// integer values of the given size. Assume that any legal integer type can
@@ -821,9 +802,7 @@ class LLVM_ABI TargetLoweringBase {
/// because a mask and compare of a single bit can be handled by inverting the
/// predicate, for example:
/// (X & 8) == 8 ---> (X & 8) != 0
- virtual bool hasAndNotCompare(SDValue Y) const {
- return false;
- }
+ virtual bool hasAndNotCompare(SDValue Y) const { return false; }
/// Return true if the target has a bitwise and-not operation:
/// X = ~A & B
@@ -952,9 +931,7 @@ class LLVM_ABI TargetLoweringBase {
// By default prefer folding (abs (sub nsw x, y)) -> abds(x, y). Some targets
// may want to avoid this to prevent loss of sub_nsw pattern.
- virtual bool preferABDSToABSWithNSW(EVT VT) const {
- return true;
- }
+ virtual bool preferABDSToABSWithNSW(EVT VT) const { return true; }
// Return true if the target wants to transform Op(Splat(X)) -> Splat(Op(X))
virtual bool preferScalarizeSplat(SDNode *N) const { return true; }
@@ -991,9 +968,7 @@ class LLVM_ABI TargetLoweringBase {
/// Return true if inserting a scalar into a variable element of an undef
/// vector is more efficiently handled by splatting the scalar instead.
- virtual bool shouldSplatInsEltVarIndex(EVT) const {
- return false;
- }
+ virtual bool shouldSplatInsEltVarIndex(EVT) const { return false; }
/// Return true if target always benefits from combining into FMA for a
/// given value type. This must typically return false on targets where FMA
@@ -1012,8 +987,7 @@ class LLVM_ABI TargetLoweringBase {
/// Return the ValueType for comparison libcalls. Comparison libcalls include
/// floating point comparison calls, and Ordered/Unordered check calls on
/// floating point numbers.
- virtual
- MVT::SimpleValueType getCmpLibcallReturnType() const;
+ virtual MVT::SimpleValueType getCmpLibcallReturnType() const;
/// For targets without i1 registers, this gives the nature of the high-bits
/// of boolean values held in types wider than i1.
@@ -1066,7 +1040,8 @@ class LLVM_ABI TargetLoweringBase {
/// Return the register class that should be used for the specified value
/// type.
- virtual const TargetRegisterClass *getRegClassFor(MVT VT, bool isDivergent = false) const {
+ virtual const TargetRegisterClass *
+ getRegClassFor(MVT VT, bool isDivergent = false) const {
(void)isDivergent;
const TargetRegisterClass *RC = RegClassForVT[VT.SimpleTy];
assert(RC && "This value type is not natively supported!");
@@ -1224,8 +1199,8 @@ class LLVM_ABI TargetLoweringBase {
}
struct IntrinsicInfo {
- unsigned opc = 0; // target opcode
- EVT memVT; // memory VT
+ unsigned opc = 0; // target opcode
+ EVT memVT; // memory VT
// value representing memory location
PointerUnion<const Value *, const PseudoSourceValue *> ptrVal;
@@ -1234,10 +1209,10 @@ class LLVM_ABI TargetLoweringBase {
// unknown address space.
std::optional<unsigned> fallbackAddressSpace;
- int offset = 0; // offset off of ptrVal
- uint64_t size = 0; // the size of the memory location
- // (taken from memVT if zero)
- MaybeAlign align = Align(1); // alignment
+ int offset = 0; // offset off of ptrVal
+ uint64_t size = 0; // the size of the memory location
+ // (taken from memVT if zero)
+ MaybeAlign align = Align(1); // alignment
MachineMemOperand::Flags flags = MachineMemOperand::MONone;
SyncScope::ID ssid = SyncScope::System;
@@ -1348,11 +1323,16 @@ class LLVM_ABI TargetLoweringBase {
LegalizeAction getStrictFPOperationAction(unsigned Op, EVT VT) const {
unsigned EqOpc;
switch (Op) {
- default: llvm_unreachable("Unexpected FP pseudo-opcode");
+ default:
+ llvm_unreachable("Unexpected FP pseudo-opcode");
#define DAG_INSTRUCTION(NAME, NARG, ROUND_MODE, INTRINSIC, DAGN) \
- case ISD::STRICT_##DAGN: EqOpc = ISD::DAGN; break;
+ case ISD::STRICT_##DAGN: \
+ EqOpc = ISD::DAGN; \
+ break;
#define CMP_INSTRUCTION(NAME, NARG, ROUND_MODE, INTRINSIC, DAGN) \
- case ISD::STRICT_##DAGN: EqOpc = ISD::SETCC; break;
+ case ISD::STRICT_##DAGN: \
+ EqOpc = ISD::SETCC; \
+ break;
#include "llvm/IR/ConstrainedOps.def"
}
@@ -1369,8 +1349,8 @@ class LLVM_ABI TargetLoweringBase {
return isOperationLegal(Op, VT);
return (VT == MVT::Other || isTypeLegal(VT)) &&
- (getOperationAction(Op, VT) == Legal ||
- getOperationAction(Op, VT) == Custom);
+ (getOperationAction(Op, VT) == Legal ||
+ getOperationAction(Op, VT) == Custom);
}
/// Return true if the specified operation is legal on this target or can be
@@ -1383,8 +1363,8 @@ class LLVM_ABI TargetLoweringBase {
return isOperationLegal(Op, VT);
return (VT == MVT::Other || isTypeLegal(VT)) &&
- (getOperationAction(Op, VT) == Legal ||
- getOperationAction(Op, VT) == Promote);
+ (getOperationAction(Op, VT) == Legal ||
+ getOperationAction(Op, VT) == Promote);
}
/// Return true if the specified operation is legal on this target or can be
@@ -1397,9 +1377,9 @@ class LLVM_ABI TargetLoweringBase {
return isOperationLegal(Op, VT);
return (VT == MVT::Other || isTypeLegal(VT)) &&
- (getOperationAction(Op, VT) == Legal ||
- getOperationAction(Op, VT) == Custom ||
- getOperationAction(Op, VT) == Promote);
+ (getOperationAction(Op, VT) == Legal ||
+ getOperationAction(Op, VT) == Custom ||
+ getOperationAction(Op, VT) == Promote);
}
/// Return true if the operation uses custom lowering, regardless of whether
@@ -1479,33 +1459,39 @@ 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;
+ LegalizeAction getLoadExtAction(unsigned ExtType, EVT ValVT, EVT MemVT,
+ unsigned AddrSpace = 0) 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);
+ return (
+ LegalizeAction)((LoadExtActions.at(AddrSpace)[ValI][MemI] >> Shift) &
+ 0xf);
}
/// 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 isLoadExtLegal(unsigned ExtType, EVT ValVT, EVT MemVT,
+ unsigned AddrSpace = 0) const {
+ return getLoadExtAction(ExtType, ValVT, MemVT, AddrSpace) == 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;
+ bool isLoadExtLegalOrCustom(unsigned ExtType, EVT ValVT, EVT MemVT,
+ unsigned AddrSpace = 0) const {
+ return getLoadExtAction(ExtType, ValVT, MemVT, AddrSpace) == Legal ||
+ getLoadExtAction(ExtType, ValVT, MemVT, AddrSpace) == Custom;
}
/// Same as getLoadExtAction, but for atomic loads.
LegalizeAction getAtomicLoadExtAction(unsigned ExtType, EVT ValVT,
EVT MemVT) const {
- if (ValVT.isExtended() || MemVT.isExtended()) return Expand;
+ 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 &&
@@ -1528,9 +1514,10 @@ class LLVM_ABI TargetLoweringBase {
/// 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 getTruncStoreAction(EVT ValVT, EVT MemVT) const {
- if (ValVT.isExtended() || MemVT.isExtended()) return Expand;
- unsigned ValI = (unsigned) ValVT.getSimpleVT().SimpleTy;
- unsigned MemI = (unsigned) MemVT.getSimpleVT().SimpleTy;
+ if (ValVT.isExtended() || MemVT.isExtended())
+ return Expand;
+ unsigned ValI = (unsigned)ValVT.getSimpleVT().SimpleTy;
+ unsigned MemI = (unsigned)MemVT.getSimpleVT().SimpleTy;
assert(ValI < MVT::VALUETYPE_SIZE && MemI < MVT::VALUETYPE_SIZE &&
"Table isn't big enough!");
return TruncStoreActions[ValI][MemI];
@@ -1545,9 +1532,8 @@ class LLVM_ABI TargetLoweringBase {
/// Return true if the specified store with truncation has solution on this
/// target.
bool isTruncStoreLegalOrCustom(EVT ValVT, EVT MemVT) const {
- return isTypeLegal(ValVT) &&
- (getTruncStoreAction(ValVT, MemVT) == Legal ||
- getTruncStoreAction(ValVT, MemVT) == Custom);
+ return isTypeLegal(ValVT) && (getTruncStoreAction(ValVT, MemVT) == Legal ||
+ getTruncStoreAction(ValVT, MemVT) == Custom);
}
virtual bool canCombineTruncStore(EVT ValVT, EVT MemVT,
@@ -1568,8 +1554,8 @@ class LLVM_ABI TargetLoweringBase {
/// Return true if the specified indexed load is legal on this target.
bool isIndexedLoadLegal(unsigned IdxMode, EVT VT) const {
return VT.isSimple() &&
- (getIndexedLoadAction(IdxMode, VT.getSimpleVT()) == Legal ||
- getIndexedLoadAction(IdxMode, VT.getSimpleVT()) == Custom);
+ (getIndexedLoadAction(IdxMode, VT.getSimpleVT()) == Legal ||
+ getIndexedLoadAction(IdxMode, VT.getSimpleVT()) == Custom);
}
/// Return how the indexed store should be treated: either it is legal, needs
@@ -1582,8 +1568,8 @@ class LLVM_ABI TargetLoweringBase {
/// Return true if the specified indexed load is legal on this target.
bool isIndexedStoreLegal(unsigned IdxMode, EVT VT) const {
return VT.isSimple() &&
- (getIndexedStoreAction(IdxMode, VT.getSimpleVT()) == Legal ||
- getIndexedStoreAction(IdxMode, VT.getSimpleVT()) == Custom);
+ (getIndexedStoreAction(IdxMode, VT.getSimpleVT()) == Legal ||
+ getIndexedStoreAction(IdxMode, VT.getSimpleVT()) == Custom);
}
/// Return how the indexed load should be treated: either ...
[truncated]
|
So we can store actions per address space. The default space, zero, is used is one is not supplied.
sparker-arm
force-pushed
the
ext-load-action-addrspace
branch
from
24d2f4a to
7fb2ce4
Compare
Sorry, I've removed that commit. |
|
Behavior right now is exactly the same as before, so it works in that respect (I checked that it doesn't break my code). Maybe I'm still missing something, but I don't see how having a default parameter helps us. Once a Custom handler is set for address space 1, the existing code will still see that the default one (0) is still "Legal", and make incorrect assumptions. So then all the (indirect) uses of getLoadExtAction (including uses isLoadExtLegal[OrCustom] of) need to be made address space aware. And then to make sure that those changes work correctly, any backends using extloads from anything other than address space 0 need fixing (not many, I imagine). Some places are easy to get an address space for, but what about something like InstructionCost
getCastInstrCost(unsigned Opcode, Type *Dst, Type *Src,
TTI::CastContextHint CCH, TTI::TargetCostKind CostKind,
const Instruction *I = nullptr) const override {llvm-project/llvm/include/llvm/CodeGen/BasicTTIImpl.h Lines 1240 to 1251 in 3ce1656 |
sparker-arm
changed the title
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions h,cpp -- llvm/include/llvm/CodeGen/BasicTTIImpl.h llvm/include/llvm/CodeGen/TargetLowering.h llvm/lib/CodeGen/CodeGenPrepare.cpp llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp llvm/lib/CodeGen/TargetLoweringBase.cpp
View the diff from clang-format here.diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index 2f9f6f8ce..52223cc85 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -1493,9 +1493,8 @@ public:
0xf);
} else {
assert(AddrSpace != 0 && "addrspace zero should be initialized");
- return (
- LegalizeAction)((LoadExtActions.at(0)[ValI][MemI] >> Shift) &
- 0xf);
+ return (LegalizeAction)((LoadExtActions.at(0)[ValI][MemI] >> Shift) &
+ 0xf);
}
}
@@ -3766,7 +3765,8 @@ private:
/// for each of the 4 load ext types. These actions can be specified for each
/// address space.
using LoadExtActionMapTy =
- std::array<std::array<uint16_t, MVT::VALUETYPE_SIZE>, MVT::VALUETYPE_SIZE>;
+ std::array<std::array<uint16_t, MVT::VALUETYPE_SIZE>,
+ MVT::VALUETYPE_SIZE>;
using LoadExtActionMap = std::map<unsigned, LoadExtActionMapTy>;
LoadExtActionMap LoadExtActions;
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index ea1f57252..eeba0ad2a 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -6850,9 +6850,8 @@ bool DAGCombiner::isAndLoadExtLoad(ConstantSDNode *AndC, LoadSDNode *LoadN,
if (!LoadedVT.bitsGT(ExtVT) || !ExtVT.isRound())
return false;
- if (LegalOperations &&
- !TLI.isLoadExtLegal(ISD::ZEXTLOAD, LoadResultTy, ExtVT,
- LoadN->getAddressSpace()))
+ if (LegalOperations && !TLI.isLoadExtLegal(ISD::ZEXTLOAD, LoadResultTy, ExtVT,
+ LoadN->getAddressSpace()))
return false;
if (!TLI.shouldReduceLoadWidth(LoadN, ISD::ZEXTLOAD, ExtVT, /*ByteOffset=*/0))
@@ -6914,9 +6913,8 @@ bool DAGCombiner::isLegalNarrowLdSt(LSBaseSDNode *LDST,
if (!SDValue(Load, 0).hasOneUse())
return false;
- if (LegalOperations &&
- !TLI.isLoadExtLegal(ExtType, Load->getValueType(0), MemVT,
- Load->getAddressSpace()))
+ if (LegalOperations && !TLI.isLoadExtLegal(ExtType, Load->getValueType(0),
+ MemVT, Load->getAddressSpace()))
return false;
// For the transform to be legal, the load must produce only two values
@@ -7581,8 +7579,8 @@ SDValue DAGCombiner::visitAND(SDNode *N) {
// 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(), Load->getAddressSpace());
+ TLI.isLoadExtLegal(ISD::ZEXTLOAD, Load->getValueType(0),
+ Load->getMemoryVT(), Load->getAddressSpace());
// 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
@@ -9698,7 +9696,7 @@ SDValue DAGCombiner::MatchLoadCombine(SDNode *N) {
// 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) {
- for (auto L : Loads) {
+ for (auto L : Loads) {
if (!TLI.isLoadExtLegal(NeedsZext ? ISD::ZEXTLOAD : ISD::NON_EXTLOAD, VT,
MemVT, L->getAddressSpace()))
return SDValue();
@@ -13313,11 +13311,11 @@ 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 &&
+ if (LHS.getOpcode() == ISD::LOAD && LHS.hasOneUse() && SetCCWidth != 1 &&
+ SetCCWidth < WideWidth &&
TLI.isLoadExtLegalOrCustom(
- LoadExtOpcode, WideVT, NarrowVT,
- cast<LoadSDNode>(LHS)->getAddressSpace()) &&
+ LoadExtOpcode, WideVT, NarrowVT,
+ cast<LoadSDNode>(LHS)->getAddressSpace()) &&
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:
@@ -13765,9 +13763,9 @@ static SDValue tryToFoldExtendSelectLoad(SDNode *N, const TargetLowering &TLI,
LoadSDNode *Load1 = cast<LoadSDNode>(Op1);
LoadSDNode *Load2 = cast<LoadSDNode>(Op2);
if (!TLI.isLoadExtLegal(ExtLoadOpcode, VT, Load1->getMemoryVT(),
- Load1->getAddressSpace()) ||
+ Load1->getAddressSpace()) ||
!TLI.isLoadExtLegal(ExtLoadOpcode, VT, Load2->getMemoryVT(),
- Load2->getAddressSpace()) ||
+ Load2->getAddressSpace()) ||
(N0->getOpcode() == ISD::VSELECT && Level >= AfterLegalizeTypes &&
TLI.getOperationAction(ISD::VSELECT, VT) != TargetLowering::Legal))
return SDValue();
@@ -13992,7 +13990,7 @@ SDValue DAGCombiner::CombineExtLoad(SDNode *N) {
EVT SplitSrcVT = SrcVT;
EVT SplitDstVT = DstVT;
while (!TLI.isLoadExtLegalOrCustom(ExtType, SplitDstVT, SplitSrcVT,
- LN0->getAddressSpace()) &&
+ LN0->getAddressSpace()) &&
SplitSrcVT.getVectorNumElements() > 1) {
SplitDstVT = DAG.GetSplitDestVTs(SplitDstVT).first;
SplitSrcVT = DAG.GetSplitDestVTs(SplitSrcVT).first;
@@ -14180,8 +14178,7 @@ static SDValue tryToFoldExtOfExtload(SelectionDAG &DAG, DAGCombiner &Combiner,
LoadSDNode *LN0 = cast<LoadSDNode>(N0);
EVT MemVT = LN0->getMemoryVT();
- if ((LegalOperations || !LN0->isSimple() ||
- VT.isVector()) &&
+ if ((LegalOperations || !LN0->isSimple() || VT.isVector()) &&
!TLI.isLoadExtLegal(ExtLoadType, VT, MemVT, LN0->getAddressSpace()))
return SDValue();
@@ -14228,10 +14225,9 @@ static SDValue tryToFoldExtOfLoad(SelectionDAG &DAG, DAGCombiner &Combiner,
// TODO: isFixedLengthVector() should be removed and any negative effects on
// code generation being the result of that target's implementation of
// isVectorLoadExtDesirable().
- if ((LegalOperations || VT.isFixedLengthVector() ||
- LN0->isSimple()) &&
+ if ((LegalOperations || VT.isFixedLengthVector() || LN0->isSimple()) &&
!TLI.isLoadExtLegal(ExtLoadType, VT, N0.getValueType(),
- LN0->getAddressSpace()))
+ LN0->getAddressSpace()))
return {};
bool DoXform = true;
@@ -14274,7 +14270,7 @@ tryToFoldExtOfMaskedLoad(SelectionDAG &DAG, const TargetLowering &TLI, EVT VT,
if ((LegalOperations || !cast<MaskedLoadSDNode>(N0)->isSimple()) &&
!TLI.isLoadExtLegalOrCustom(ExtLoadType, VT, Ld->getValueType(0),
- Ld->getAddressSpace()))
+ Ld->getAddressSpace()))
return SDValue();
if (!TLI.isVectorLoadExtDesirable(SDValue(N, 0)))
@@ -14419,7 +14415,7 @@ SDValue DAGCombiner::foldSextSetcc(SDNode *N) {
ISD::isUNINDEXEDLoad(V.getNode()) &&
cast<LoadSDNode>(V)->isSimple() &&
TLI.isLoadExtLegal(LoadOpcode, VT, V.getValueType(),
- cast<LoadSDNode>(V)->getAddressSpace())))
+ cast<LoadSDNode>(V)->getAddressSpace())))
return false;
// Non-chain users of this value must either be the setcc in this
@@ -14617,7 +14613,7 @@ SDValue DAGCombiner::visitSIGN_EXTEND(SDNode *N) {
LoadSDNode *LN00 = cast<LoadSDNode>(N0.getOperand(0));
EVT MemVT = LN00->getMemoryVT();
if (TLI.isLoadExtLegal(ISD::SEXTLOAD, VT, MemVT, LN00->getAddressSpace()) &&
- LN00->getExtensionType() != ISD::ZEXTLOAD && LN00->isUnindexed()) {
+ LN00->getExtensionType() != ISD::ZEXTLOAD && LN00->isUnindexed()) {
SmallVector<SDNode*, 4> SetCCs;
bool DoXform = ExtendUsesToFormExtLoad(VT, N0.getNode(), N0.getOperand(0),
ISD::SIGN_EXTEND, SetCCs, TLI);
@@ -15165,8 +15161,9 @@ SDValue DAGCombiner::visitANY_EXTEND(SDNode *N) {
return foldedExt;
} else if (ISD::isNON_EXTLoad(N0.getNode()) &&
ISD::isUNINDEXEDLoad(N0.getNode()) &&
- TLI.isLoadExtLegalOrCustom(ISD::EXTLOAD, VT, N0.getValueType(),
- cast<LoadSDNode>(N0)->getAddressSpace())) {
+ TLI.isLoadExtLegalOrCustom(
+ ISD::EXTLOAD, VT, N0.getValueType(),
+ cast<LoadSDNode>(N0)->getAddressSpace())) {
bool DoXform = true;
SmallVector<SDNode *, 4> SetCCs;
if (!N0.hasOneUse())
@@ -15754,7 +15751,7 @@ SDValue DAGCombiner::visitSIGN_EXTEND_INREG(SDNode *N) {
((!LegalOperations && cast<LoadSDNode>(N0)->isSimple() &&
N0.hasOneUse()) ||
TLI.isLoadExtLegal(ISD::SEXTLOAD, VT, ExtVT,
- cast<LoadSDNode>(N0)->getAddressSpace()))) {
+ cast<LoadSDNode>(N0)->getAddressSpace()))) {
auto *LN0 = cast<LoadSDNode>(N0);
SDValue ExtLoad =
DAG.getExtLoad(ISD::SEXTLOAD, DL, VT, LN0->getChain(),
@@ -15785,8 +15782,7 @@ SDValue DAGCombiner::visitSIGN_EXTEND_INREG(SDNode *N) {
if (MaskedLoadSDNode *Ld = dyn_cast<MaskedLoadSDNode>(N0)) {
if (ExtVT == Ld->getMemoryVT() && N0.hasOneUse() &&
Ld->getExtensionType() != ISD::LoadExtType::NON_EXTLOAD &&
- TLI.isLoadExtLegal(ISD::SEXTLOAD, VT, ExtVT,
- Ld->getAddressSpace())) {
+ TLI.isLoadExtLegal(ISD::SEXTLOAD, VT, ExtVT, Ld->getAddressSpace())) {
SDValue ExtMaskedLoad = DAG.getMaskedLoad(
VT, DL, Ld->getChain(), Ld->getBasePtr(), Ld->getOffset(),
Ld->getMask(), Ld->getPassThru(), ExtVT, Ld->getMemOperand(),
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
index f66ab797f..9b013503a 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
@@ -743,7 +743,8 @@ void SelectionDAGLegalize::LegalizeLoadOps(SDNode *Node) {
// Until such a way is found, don't insist on promoting i1 here.
(SrcVT != MVT::i1 ||
TLI.getLoadExtAction(ExtType, Node->getValueType(0), MVT::i1,
- LD->getAddressSpace()) == TargetLowering::Promote)) {
+ LD->getAddressSpace()) ==
+ TargetLowering::Promote)) {
// Promote to a byte-sized load if not loading an integral number of
// bytes. For example, promote EXTLOAD:i20 -> EXTLOAD:i24.
unsigned NewWidth = SrcVT.getStoreSizeInBits();
|
I think this is all done now. I've fixed up the AMDGPU tests, as I figured that would be the best backend to begin with. I haven't figured out why the tests change though, but there aren't many. I do get a hard crash with a AArch64 SVE test though!
Yeah, the main problem with this, current, implementation is that once you set an action for an explicit (non-zero) address space, I think this means that actions need to be set for all types :/ |
Actually, it goes further. If you set an action other than Legal for even addrspace 0, you have to set for every other possible address space (unless of course, the operation would be invalid) in order to maintain the current behavior. That's why all the tests for AMDGPU were messed up. I'm looking at it, and the modified tests are all loads from non-standard address spaces. The tests had the correct behavior. The problem is that Promote and Expand are only being triggered for addrspace(0) now, not the other two or more addrspaces being tested for the behavior. It's the implementation of As for making all uses aware of address spaces, you didn't finish it ALL yet. You missed
Other than those three, I think you got the rest. About the crashes, looking at the logs from the test runner, it's not just the SVE test. Several other tests crash at It probably comes back down to the fact that you didn't finish modifying for (MVT VT : MVT::integer_fixedlen_vector_valuetypes())
for (auto MemVT :
{MVT::v2i8, MVT::v4i8, MVT::v2i16, MVT::v3i16, MVT::v4i16})
setLoadExtAction({ISD::SEXTLOAD, ISD::ZEXTLOAD, ISD::EXTLOAD}, VT,
MemVT, Expand);Should be able to ignore the crash for now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
So we can store actions per address space.