[CodeGen] Use LocationSize for MMO getSize #84751
Merged
+277
−206
Conversation
This file contains 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
davemgreen
requested review from
aemerson,
arsenm,
nikic,
RKSimon,
topperc and
harviniriawan
llvmbot
added
backend:ARM
backend:AArch64
backend:AMDGPU
backend:RISC-V
backend:PowerPC
backend:SystemZ
backend:X86
debuginfo
llvm:globalisel
llvm:SelectionDAG
|
@llvm/pr-subscribers-backend-x86 @llvm/pr-subscribers-llvm-globalisel Author: David Green (davemgreen) ChangesThis is part of #70452 that changes the type used for the external interface of MMO to LocationSize as opposed to uint64_t. This means the constructors take LocationSize, and convert ~UINT64_C(0) to LocationSize::beforeOrAfter(). The getSize methods return a LocationSize. This allows us to be more precise with unknown sizes, not accidentally treating them as unsigned values, and in the future should allow us to add proper scalable vector support but none of that is included in this patch. It should mostly be an NFC. Global ISel is still expected to use the underlying LLT as it needs, and are not expected to see unknown sizes for generic operations. Most of the changes are hopefully fairly mechanical, adding a lot of getValue() calls and protecting them with hasValue() where needed. Patch is 78.92 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/84751.diff 48 Files Affected:
diff --git a/llvm/include/llvm/Analysis/MemoryLocation.h b/llvm/include/llvm/Analysis/MemoryLocation.h
index b72a27cab86b34..6d3d0acc2076d4 100644
--- a/llvm/include/llvm/Analysis/MemoryLocation.h
+++ b/llvm/include/llvm/Analysis/MemoryLocation.h
@@ -191,8 +191,14 @@ class LocationSize {
return Value == Other.Value;
}
+ bool operator==(const TypeSize &Other) const {
+ return hasValue() && getValue() == Other;
+ }
+
bool operator!=(const LocationSize &Other) const { return !(*this == Other); }
+ bool operator!=(const TypeSize &Other) const { return !(*this == Other); }
+
// Ordering operators are not provided, since it's unclear if there's only one
// reasonable way to compare:
// - values that don't exist against values that do, and
@@ -293,8 +299,9 @@ class MemoryLocation {
// Return the exact size if the exact size is known at compiletime,
// otherwise return MemoryLocation::UnknownSize.
- static uint64_t getSizeOrUnknown(const TypeSize &T) {
- return T.isScalable() ? UnknownSize : T.getFixedValue();
+ static LocationSize getSizeOrUnknown(const TypeSize &T) {
+ return T.isScalable() ? LocationSize::beforeOrAfterPointer()
+ : LocationSize::precise(T.getFixedValue());
}
MemoryLocation() : Ptr(nullptr), Size(LocationSize::beforeOrAfterPointer()) {}
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
index 8e456015cd3736..c73ac2c9f55b7b 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
@@ -647,15 +647,15 @@ bool GIMatchTableExecutor::executeMatchTable(
unsigned Size = MRI.getType(MO.getReg()).getSizeInBits();
if (MatcherOpcode == GIM_CheckMemorySizeEqualToLLT &&
- MMO->getSizeInBits() != Size) {
+ MMO->getSizeInBits().getValue() != Size) {
if (handleReject() == RejectAndGiveUp)
return false;
} else if (MatcherOpcode == GIM_CheckMemorySizeLessThanLLT &&
- MMO->getSizeInBits() >= Size) {
+ MMO->getSizeInBits().getValue() >= Size) {
if (handleReject() == RejectAndGiveUp)
return false;
} else if (MatcherOpcode == GIM_CheckMemorySizeGreaterThanLLT &&
- MMO->getSizeInBits() <= Size)
+ MMO->getSizeInBits().getValue() <= Size)
if (handleReject() == RejectAndGiveUp)
return false;
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h b/llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h
index f5a6528d10a973..1d2c45a12b268a 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h
@@ -54,9 +54,9 @@ class GMemOperation : public GenericMachineInstr {
bool isUnordered() const { return getMMO().isUnordered(); }
/// Returns the size in bytes of the memory access.
- uint64_t getMemSize() const { return getMMO().getSize(); }
+ LocationSize getMemSize() const { return getMMO().getSize(); }
/// Returns the size in bits of the memory access.
- uint64_t getMemSizeInBits() const { return getMMO().getSizeInBits(); }
+ LocationSize getMemSizeInBits() const { return getMMO().getSizeInBits(); }
static bool classof(const MachineInstr *MI) {
return GenericMachineInstr::classof(MI) && MI->hasOneMemOperand();
diff --git a/llvm/include/llvm/CodeGen/MachineFunction.h b/llvm/include/llvm/CodeGen/MachineFunction.h
index a2c90c9f42f38f..dfbf7a1e7aae53 100644
--- a/llvm/include/llvm/CodeGen/MachineFunction.h
+++ b/llvm/include/llvm/CodeGen/MachineFunction.h
@@ -1026,18 +1026,27 @@ class LLVM_EXTERNAL_VISIBILITY MachineFunction {
/// MachineMemOperands are owned by the MachineFunction and need not be
/// explicitly deallocated.
MachineMemOperand *getMachineMemOperand(
- MachinePointerInfo PtrInfo, MachineMemOperand::Flags f, uint64_t s,
+ MachinePointerInfo PtrInfo, MachineMemOperand::Flags f, LLT MemTy,
Align base_alignment, const AAMDNodes &AAInfo = AAMDNodes(),
const MDNode *Ranges = nullptr, SyncScope::ID SSID = SyncScope::System,
AtomicOrdering Ordering = AtomicOrdering::NotAtomic,
AtomicOrdering FailureOrdering = AtomicOrdering::NotAtomic);
-
MachineMemOperand *getMachineMemOperand(
- MachinePointerInfo PtrInfo, MachineMemOperand::Flags f, LLT MemTy,
- Align base_alignment, const AAMDNodes &AAInfo = AAMDNodes(),
+ MachinePointerInfo PtrInfo, MachineMemOperand::Flags F, LocationSize Size,
+ Align BaseAlignment, const AAMDNodes &AAInfo = AAMDNodes(),
const MDNode *Ranges = nullptr, SyncScope::ID SSID = SyncScope::System,
AtomicOrdering Ordering = AtomicOrdering::NotAtomic,
AtomicOrdering FailureOrdering = AtomicOrdering::NotAtomic);
+ MachineMemOperand *getMachineMemOperand(
+ MachinePointerInfo PtrInfo, MachineMemOperand::Flags F, uint64_t Size,
+ Align BaseAlignment, const AAMDNodes &AAInfo = AAMDNodes(),
+ const MDNode *Ranges = nullptr, SyncScope::ID SSID = SyncScope::System,
+ AtomicOrdering Ordering = AtomicOrdering::NotAtomic,
+ AtomicOrdering FailureOrdering = AtomicOrdering::NotAtomic) {
+ return getMachineMemOperand(PtrInfo, F, LocationSize::precise(Size),
+ BaseAlignment, AAInfo, Ranges, SSID, Ordering,
+ FailureOrdering);
+ }
/// getMachineMemOperand - Allocate a new MachineMemOperand by copying
/// an existing one, adjusting by an offset and using the given size.
@@ -1046,9 +1055,16 @@ class LLVM_EXTERNAL_VISIBILITY MachineFunction {
MachineMemOperand *getMachineMemOperand(const MachineMemOperand *MMO,
int64_t Offset, LLT Ty);
MachineMemOperand *getMachineMemOperand(const MachineMemOperand *MMO,
- int64_t Offset, uint64_t Size) {
+ int64_t Offset, LocationSize Size) {
return getMachineMemOperand(
- MMO, Offset, Size == ~UINT64_C(0) ? LLT() : LLT::scalar(8 * Size));
+ MMO, Offset,
+ !Size.hasValue() || Size.isScalable()
+ ? LLT()
+ : LLT::scalar(8 * Size.getValue().getKnownMinValue()));
+ }
+ MachineMemOperand *getMachineMemOperand(const MachineMemOperand *MMO,
+ int64_t Offset, uint64_t Size) {
+ return getMachineMemOperand(MMO, Offset, LocationSize::precise(Size));
}
/// getMachineMemOperand - Allocate a new MachineMemOperand by copying
@@ -1057,10 +1073,15 @@ class LLVM_EXTERNAL_VISIBILITY MachineFunction {
/// explicitly deallocated.
MachineMemOperand *getMachineMemOperand(const MachineMemOperand *MMO,
const MachinePointerInfo &PtrInfo,
- uint64_t Size);
+ LocationSize Size);
MachineMemOperand *getMachineMemOperand(const MachineMemOperand *MMO,
const MachinePointerInfo &PtrInfo,
LLT Ty);
+ MachineMemOperand *getMachineMemOperand(const MachineMemOperand *MMO,
+ const MachinePointerInfo &PtrInfo,
+ uint64_t Size) {
+ return getMachineMemOperand(MMO, PtrInfo, LocationSize::precise(Size));
+ }
/// Allocate a new MachineMemOperand by copying an existing one,
/// replacing only AliasAnalysis information. MachineMemOperands are owned
diff --git a/llvm/include/llvm/CodeGen/MachineMemOperand.h b/llvm/include/llvm/CodeGen/MachineMemOperand.h
index 12c18aaea5b26c..da4ca582cb9e4f 100644
--- a/llvm/include/llvm/CodeGen/MachineMemOperand.h
+++ b/llvm/include/llvm/CodeGen/MachineMemOperand.h
@@ -17,6 +17,7 @@
#include "llvm/ADT/BitmaskEnum.h"
#include "llvm/ADT/PointerUnion.h"
+#include "llvm/Analysis/MemoryLocation.h"
#include "llvm/CodeGen/PseudoSourceValue.h"
#include "llvm/CodeGenTypes/LowLevelType.h"
#include "llvm/IR/DerivedTypes.h"
@@ -186,7 +187,7 @@ class MachineMemOperand {
/// and atomic ordering requirements must also be specified. For cmpxchg
/// atomic operations the atomic ordering requirements when store does not
/// occur must also be specified.
- MachineMemOperand(MachinePointerInfo PtrInfo, Flags flags, uint64_t s,
+ MachineMemOperand(MachinePointerInfo PtrInfo, Flags flags, LocationSize TS,
Align a, const AAMDNodes &AAInfo = AAMDNodes(),
const MDNode *Ranges = nullptr,
SyncScope::ID SSID = SyncScope::System,
@@ -235,13 +236,17 @@ class MachineMemOperand {
LLT getMemoryType() const { return MemoryType; }
/// Return the size in bytes of the memory reference.
- uint64_t getSize() const {
- return MemoryType.isValid() ? MemoryType.getSizeInBytes() : ~UINT64_C(0);
+ LocationSize getSize() const {
+ return MemoryType.isValid()
+ ? LocationSize::precise(MemoryType.getSizeInBytes())
+ : LocationSize::beforeOrAfterPointer();
}
/// Return the size in bits of the memory reference.
- uint64_t getSizeInBits() const {
- return MemoryType.isValid() ? MemoryType.getSizeInBits() : ~UINT64_C(0);
+ LocationSize getSizeInBits() const {
+ return MemoryType.isValid()
+ ? LocationSize::precise(MemoryType.getSizeInBits())
+ : LocationSize::beforeOrAfterPointer();
}
LLT getType() const {
diff --git a/llvm/include/llvm/CodeGen/SelectionDAG.h b/llvm/include/llvm/CodeGen/SelectionDAG.h
index 25e6c525b672a1..4785e93d72d1cc 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAG.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAG.h
@@ -1299,7 +1299,7 @@ class SelectionDAG {
EVT MemVT, MachinePointerInfo PtrInfo, Align Alignment,
MachineMemOperand::Flags Flags = MachineMemOperand::MOLoad |
MachineMemOperand::MOStore,
- uint64_t Size = 0, const AAMDNodes &AAInfo = AAMDNodes());
+ LocationSize Size = 0, const AAMDNodes &AAInfo = AAMDNodes());
inline SDValue getMemIntrinsicNode(
unsigned Opcode, const SDLoc &dl, SDVTList VTList, ArrayRef<SDValue> Ops,
@@ -1307,7 +1307,7 @@ class SelectionDAG {
MaybeAlign Alignment = std::nullopt,
MachineMemOperand::Flags Flags = MachineMemOperand::MOLoad |
MachineMemOperand::MOStore,
- uint64_t Size = 0, const AAMDNodes &AAInfo = AAMDNodes()) {
+ LocationSize Size = 0, const AAMDNodes &AAInfo = AAMDNodes()) {
// Ensure that codegen never sees alignment 0
return getMemIntrinsicNode(Opcode, dl, VTList, Ops, MemVT, PtrInfo,
Alignment.value_or(getEVTAlign(MemVT)), Flags,
diff --git a/llvm/lib/CodeGen/DFAPacketizer.cpp b/llvm/lib/CodeGen/DFAPacketizer.cpp
index 48bb4a07662e10..c16166a1d5e1c5 100644
--- a/llvm/lib/CodeGen/DFAPacketizer.cpp
+++ b/llvm/lib/CodeGen/DFAPacketizer.cpp
@@ -252,12 +252,13 @@ void VLIWPacketizerList::PacketizeMIs(MachineBasicBlock *MBB,
bool VLIWPacketizerList::alias(const MachineMemOperand &Op1,
const MachineMemOperand &Op2,
bool UseTBAA) const {
- if (!Op1.getValue() || !Op2.getValue())
+ if (!Op1.getValue() || !Op2.getValue() || !Op1.getSize().hasValue() ||
+ !Op2.getSize().hasValue())
return true;
int64_t MinOffset = std::min(Op1.getOffset(), Op2.getOffset());
- int64_t Overlapa = Op1.getSize() + Op1.getOffset() - MinOffset;
- int64_t Overlapb = Op2.getSize() + Op2.getOffset() - MinOffset;
+ int64_t Overlapa = Op1.getSize().getValue() + Op1.getOffset() - MinOffset;
+ int64_t Overlapb = Op2.getSize().getValue() + Op2.getOffset() - MinOffset;
AliasResult AAResult =
AA->alias(MemoryLocation(Op1.getValue(), Overlapa,
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index ab055b723dbb1f..79541490ce7412 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -770,12 +770,12 @@ bool CombinerHelper::matchCombineLoadWithAndMask(MachineInstr &MI,
LLT RegTy = MRI.getType(LoadReg);
Register PtrReg = LoadMI->getPointerReg();
unsigned RegSize = RegTy.getSizeInBits();
- uint64_t LoadSizeBits = LoadMI->getMemSizeInBits();
+ LocationSize LoadSizeBits = LoadMI->getMemSizeInBits();
unsigned MaskSizeBits = MaskVal.countr_one();
// The mask may not be larger than the in-memory type, as it might cover sign
// extended bits
- if (MaskSizeBits > LoadSizeBits)
+ if (MaskSizeBits > LoadSizeBits.getValue())
return false;
// If the mask covers the whole destination register, there's nothing to
@@ -795,7 +795,8 @@ bool CombinerHelper::matchCombineLoadWithAndMask(MachineInstr &MI,
// still adjust the opcode to indicate the high bit behavior.
if (LoadMI->isSimple())
MemDesc.MemoryTy = LLT::scalar(MaskSizeBits);
- else if (LoadSizeBits > MaskSizeBits || LoadSizeBits == RegSize)
+ else if (LoadSizeBits.getValue() > MaskSizeBits ||
+ LoadSizeBits.getValue() == RegSize)
return false;
// TODO: Could check if it's legal with the reduced or original memory size.
@@ -860,7 +861,8 @@ bool CombinerHelper::matchSextTruncSextLoad(MachineInstr &MI) {
if (auto *LoadMI = getOpcodeDef<GSExtLoad>(LoadUser, MRI)) {
// If truncating more than the original extended value, abort.
auto LoadSizeBits = LoadMI->getMemSizeInBits();
- if (TruncSrc && MRI.getType(TruncSrc).getSizeInBits() < LoadSizeBits)
+ if (TruncSrc &&
+ MRI.getType(TruncSrc).getSizeInBits() < LoadSizeBits.getValue())
return false;
if (LoadSizeBits == SizeInBits)
return true;
@@ -891,7 +893,7 @@ bool CombinerHelper::matchSextInRegOfLoad(
if (!LoadDef || !MRI.hasOneNonDBGUse(DstReg))
return false;
- uint64_t MemBits = LoadDef->getMemSizeInBits();
+ uint64_t MemBits = LoadDef->getMemSizeInBits().getValue();
// If the sign extend extends from a narrower width than the load's width,
// then we can narrow the load width when we combine to a G_SEXTLOAD.
diff --git a/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp b/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
index 099bf45b2734cb..2e2cc9a95bd95c 100644
--- a/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
@@ -415,7 +415,8 @@ void GISelKnownBits::computeKnownBitsImpl(Register R, KnownBits &Known,
if (DstTy.isVector())
break;
// Everything above the retrieved bits is zero
- Known.Zero.setBitsFrom((*MI.memoperands_begin())->getSizeInBits());
+ Known.Zero.setBitsFrom(
+ (*MI.memoperands_begin())->getSizeInBits().getValue());
break;
}
case TargetOpcode::G_ASHR: {
@@ -666,7 +667,7 @@ unsigned GISelKnownBits::computeNumSignBits(Register R,
// e.g. i16->i32 = '17' bits known.
const MachineMemOperand *MMO = *MI.memoperands_begin();
- return TyBits - MMO->getSizeInBits() + 1;
+ return TyBits - MMO->getSizeInBits().getValue() + 1;
}
case TargetOpcode::G_ZEXTLOAD: {
// FIXME: We need an in-memory type representation.
@@ -675,7 +676,7 @@ unsigned GISelKnownBits::computeNumSignBits(Register R,
// e.g. i16->i32 = '16' bits known.
const MachineMemOperand *MMO = *MI.memoperands_begin();
- return TyBits - MMO->getSizeInBits();
+ return TyBits - MMO->getSizeInBits().getValue();
}
case TargetOpcode::G_TRUNC: {
Register Src = MI.getOperand(1).getReg();
diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index bd3ff7265d51f9..bc062041a564d1 100644
--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -1317,7 +1317,7 @@ LegalizerHelper::LegalizeResult LegalizerHelper::narrowScalar(MachineInstr &MI,
if (DstTy.isVector())
return UnableToLegalize;
- if (8 * LoadMI.getMemSize() != DstTy.getSizeInBits()) {
+ if (8 * LoadMI.getMemSize().getValue() != DstTy.getSizeInBits()) {
Register TmpReg = MRI.createGenericVirtualRegister(NarrowTy);
MIRBuilder.buildLoad(TmpReg, LoadMI.getPointerReg(), LoadMI.getMMO());
MIRBuilder.buildAnyExt(DstReg, TmpReg);
@@ -1335,7 +1335,7 @@ LegalizerHelper::LegalizeResult LegalizerHelper::narrowScalar(MachineInstr &MI,
Register TmpReg = MRI.createGenericVirtualRegister(NarrowTy);
auto &MMO = LoadMI.getMMO();
- unsigned MemSize = MMO.getSizeInBits();
+ unsigned MemSize = MMO.getSizeInBits().getValue();
if (MemSize == NarrowSize) {
MIRBuilder.buildLoad(TmpReg, PtrReg, MMO);
@@ -1368,7 +1368,7 @@ LegalizerHelper::LegalizeResult LegalizerHelper::narrowScalar(MachineInstr &MI,
if (SrcTy.isVector() && LeftoverBits != 0)
return UnableToLegalize;
- if (8 * StoreMI.getMemSize() != SrcTy.getSizeInBits()) {
+ if (8 * StoreMI.getMemSize().getValue() != SrcTy.getSizeInBits()) {
Register TmpReg = MRI.createGenericVirtualRegister(NarrowTy);
MIRBuilder.buildTrunc(TmpReg, SrcReg);
MIRBuilder.buildStore(TmpReg, StoreMI.getPointerReg(), StoreMI.getMMO());
@@ -4456,7 +4456,7 @@ LegalizerHelper::reduceLoadStoreWidth(GLoadStore &LdStMI, unsigned TypeIdx,
LLT ValTy = MRI.getType(ValReg);
// FIXME: Do we need a distinct NarrowMemory legalize action?
- if (ValTy.getSizeInBits() != 8 * LdStMI.getMemSize()) {
+ if (ValTy.getSizeInBits() != 8 * LdStMI.getMemSize().getValue()) {
LLVM_DEBUG(dbgs() << "Can't narrow extload/truncstore\n");
return UnableToLegalize;
}
diff --git a/llvm/lib/CodeGen/GlobalISel/LoadStoreOpt.cpp b/llvm/lib/CodeGen/GlobalISel/LoadStoreOpt.cpp
index b5c9d3e912cc20..9fc8ecd60b03ff 100644
--- a/llvm/lib/CodeGen/GlobalISel/LoadStoreOpt.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LoadStoreOpt.cpp
@@ -117,12 +117,8 @@ bool GISelAddressing::aliasIsKnownForLoadStore(const MachineInstr &MI1,
if (!BasePtr0.BaseReg.isValid() || !BasePtr1.BaseReg.isValid())
return false;
- LocationSize Size1 = LdSt1->getMemSize() != MemoryLocation::UnknownSize
- ? LdSt1->getMemSize()
- : LocationSize::beforeOrAfterPointer();
- LocationSize Size2 = LdSt2->getMemSize() != MemoryLocation::UnknownSize
- ? LdSt2->getMemSize()
- : LocationSize::beforeOrAfterPointer();
+ LocationSize Size1 = LdSt1->getMemSize();
+ LocationSize Size2 = LdSt2->getMemSize();
int64_t PtrDiff;
if (BasePtr0.BaseReg == BasePtr1.BaseReg) {
@@ -214,14 +210,9 @@ bool GISelAddressing::instMayAlias(const MachineInstr &MI,
Offset = 0;
}
- TypeSize Size = LS->getMMO().getMemoryType().getSizeInBytes();
- return {LS->isVolatile(),
- LS->isAtomic(),
- BaseReg,
- Offset /*base offset*/,
- Size.isScalable() ? LocationSize::beforeOrAfterPointer()
- : LocationSize::precise(Size),
- &LS->getMMO()};
+ LocationSize Size = LS->getMMO().getSize();
+ return {LS->isVolatile(), LS->isAtomic(), BaseReg,
+ Offset /*base offset*/, Size, &LS->getMMO()};
}
// FIXME: support recognizing lifetime instructions.
// Default.
diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
index cfc8c28b99e562..481d9e341da377 100644
--- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
+++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
@@ -1356,10 +1356,11 @@ InstrRefBasedLDV::findLocationForMemOperand(const MachineInstr &MI) {
// from the stack at some point. Happily the memory operand will tell us
// the size written to the stack.
auto *MemOperand = *MI.memoperands_begin();
- unsigned SizeInBits = MemOperand->getSizeInBits();
+ LocationSize SizeInBits = MemOperand->getSizeInBits();
+ assert(SizeInBits.hasValue() && "Expected to find a valid size!");
// Find that position in the stack indexes we're tracking.
- auto IdxIt = MTracker->StackSlotIdxes.find({SizeInB...
[truncated]
|
|
@llvm/pr-subscribers-debuginfo Author: David Green (davemgreen) ChangesThis is part of #70452 that changes the type used for the external interface of MMO to LocationSize as opposed to uint64_t. This means the constructors take LocationSize, and convert ~UINT64_C(0) to LocationSize::beforeOrAfter(). The getSize methods return a LocationSize. This allows us to be more precise with unknown sizes, not accidentally treating them as unsigned values, and in the future should allow us to add proper scalable vector support but none of that is included in this patch. It should mostly be an NFC. Global ISel is still expected to use the underlying LLT as it needs, and are not expected to see unknown sizes for generic operations. Most of the changes are hopefully fairly mechanical, adding a lot of getValue() calls and protecting them with hasValue() where needed. Patch is 78.92 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/84751.diff 48 Files Affected:
diff --git a/llvm/include/llvm/Analysis/MemoryLocation.h b/llvm/include/llvm/Analysis/MemoryLocation.h
index b72a27cab86b34..6d3d0acc2076d4 100644
--- a/llvm/include/llvm/Analysis/MemoryLocation.h
+++ b/llvm/include/llvm/Analysis/MemoryLocation.h
@@ -191,8 +191,14 @@ class LocationSize {
return Value == Other.Value;
}
+ bool operator==(const TypeSize &Other) const {
+ return hasValue() && getValue() == Other;
+ }
+
bool operator!=(const LocationSize &Other) const { return !(*this == Other); }
+ bool operator!=(const TypeSize &Other) const { return !(*this == Other); }
+
// Ordering operators are not provided, since it's unclear if there's only one
// reasonable way to compare:
// - values that don't exist against values that do, and
@@ -293,8 +299,9 @@ class MemoryLocation {
// Return the exact size if the exact size is known at compiletime,
// otherwise return MemoryLocation::UnknownSize.
- static uint64_t getSizeOrUnknown(const TypeSize &T) {
- return T.isScalable() ? UnknownSize : T.getFixedValue();
+ static LocationSize getSizeOrUnknown(const TypeSize &T) {
+ return T.isScalable() ? LocationSize::beforeOrAfterPointer()
+ : LocationSize::precise(T.getFixedValue());
}
MemoryLocation() : Ptr(nullptr), Size(LocationSize::beforeOrAfterPointer()) {}
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
index 8e456015cd3736..c73ac2c9f55b7b 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
@@ -647,15 +647,15 @@ bool GIMatchTableExecutor::executeMatchTable(
unsigned Size = MRI.getType(MO.getReg()).getSizeInBits();
if (MatcherOpcode == GIM_CheckMemorySizeEqualToLLT &&
- MMO->getSizeInBits() != Size) {
+ MMO->getSizeInBits().getValue() != Size) {
if (handleReject() == RejectAndGiveUp)
return false;
} else if (MatcherOpcode == GIM_CheckMemorySizeLessThanLLT &&
- MMO->getSizeInBits() >= Size) {
+ MMO->getSizeInBits().getValue() >= Size) {
if (handleReject() == RejectAndGiveUp)
return false;
} else if (MatcherOpcode == GIM_CheckMemorySizeGreaterThanLLT &&
- MMO->getSizeInBits() <= Size)
+ MMO->getSizeInBits().getValue() <= Size)
if (handleReject() == RejectAndGiveUp)
return false;
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h b/llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h
index f5a6528d10a973..1d2c45a12b268a 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h
@@ -54,9 +54,9 @@ class GMemOperation : public GenericMachineInstr {
bool isUnordered() const { return getMMO().isUnordered(); }
/// Returns the size in bytes of the memory access.
- uint64_t getMemSize() const { return getMMO().getSize(); }
+ LocationSize getMemSize() const { return getMMO().getSize(); }
/// Returns the size in bits of the memory access.
- uint64_t getMemSizeInBits() const { return getMMO().getSizeInBits(); }
+ LocationSize getMemSizeInBits() const { return getMMO().getSizeInBits(); }
static bool classof(const MachineInstr *MI) {
return GenericMachineInstr::classof(MI) && MI->hasOneMemOperand();
diff --git a/llvm/include/llvm/CodeGen/MachineFunction.h b/llvm/include/llvm/CodeGen/MachineFunction.h
index a2c90c9f42f38f..dfbf7a1e7aae53 100644
--- a/llvm/include/llvm/CodeGen/MachineFunction.h
+++ b/llvm/include/llvm/CodeGen/MachineFunction.h
@@ -1026,18 +1026,27 @@ class LLVM_EXTERNAL_VISIBILITY MachineFunction {
/// MachineMemOperands are owned by the MachineFunction and need not be
/// explicitly deallocated.
MachineMemOperand *getMachineMemOperand(
- MachinePointerInfo PtrInfo, MachineMemOperand::Flags f, uint64_t s,
+ MachinePointerInfo PtrInfo, MachineMemOperand::Flags f, LLT MemTy,
Align base_alignment, const AAMDNodes &AAInfo = AAMDNodes(),
const MDNode *Ranges = nullptr, SyncScope::ID SSID = SyncScope::System,
AtomicOrdering Ordering = AtomicOrdering::NotAtomic,
AtomicOrdering FailureOrdering = AtomicOrdering::NotAtomic);
-
MachineMemOperand *getMachineMemOperand(
- MachinePointerInfo PtrInfo, MachineMemOperand::Flags f, LLT MemTy,
- Align base_alignment, const AAMDNodes &AAInfo = AAMDNodes(),
+ MachinePointerInfo PtrInfo, MachineMemOperand::Flags F, LocationSize Size,
+ Align BaseAlignment, const AAMDNodes &AAInfo = AAMDNodes(),
const MDNode *Ranges = nullptr, SyncScope::ID SSID = SyncScope::System,
AtomicOrdering Ordering = AtomicOrdering::NotAtomic,
AtomicOrdering FailureOrdering = AtomicOrdering::NotAtomic);
+ MachineMemOperand *getMachineMemOperand(
+ MachinePointerInfo PtrInfo, MachineMemOperand::Flags F, uint64_t Size,
+ Align BaseAlignment, const AAMDNodes &AAInfo = AAMDNodes(),
+ const MDNode *Ranges = nullptr, SyncScope::ID SSID = SyncScope::System,
+ AtomicOrdering Ordering = AtomicOrdering::NotAtomic,
+ AtomicOrdering FailureOrdering = AtomicOrdering::NotAtomic) {
+ return getMachineMemOperand(PtrInfo, F, LocationSize::precise(Size),
+ BaseAlignment, AAInfo, Ranges, SSID, Ordering,
+ FailureOrdering);
+ }
/// getMachineMemOperand - Allocate a new MachineMemOperand by copying
/// an existing one, adjusting by an offset and using the given size.
@@ -1046,9 +1055,16 @@ class LLVM_EXTERNAL_VISIBILITY MachineFunction {
MachineMemOperand *getMachineMemOperand(const MachineMemOperand *MMO,
int64_t Offset, LLT Ty);
MachineMemOperand *getMachineMemOperand(const MachineMemOperand *MMO,
- int64_t Offset, uint64_t Size) {
+ int64_t Offset, LocationSize Size) {
return getMachineMemOperand(
- MMO, Offset, Size == ~UINT64_C(0) ? LLT() : LLT::scalar(8 * Size));
+ MMO, Offset,
+ !Size.hasValue() || Size.isScalable()
+ ? LLT()
+ : LLT::scalar(8 * Size.getValue().getKnownMinValue()));
+ }
+ MachineMemOperand *getMachineMemOperand(const MachineMemOperand *MMO,
+ int64_t Offset, uint64_t Size) {
+ return getMachineMemOperand(MMO, Offset, LocationSize::precise(Size));
}
/// getMachineMemOperand - Allocate a new MachineMemOperand by copying
@@ -1057,10 +1073,15 @@ class LLVM_EXTERNAL_VISIBILITY MachineFunction {
/// explicitly deallocated.
MachineMemOperand *getMachineMemOperand(const MachineMemOperand *MMO,
const MachinePointerInfo &PtrInfo,
- uint64_t Size);
+ LocationSize Size);
MachineMemOperand *getMachineMemOperand(const MachineMemOperand *MMO,
const MachinePointerInfo &PtrInfo,
LLT Ty);
+ MachineMemOperand *getMachineMemOperand(const MachineMemOperand *MMO,
+ const MachinePointerInfo &PtrInfo,
+ uint64_t Size) {
+ return getMachineMemOperand(MMO, PtrInfo, LocationSize::precise(Size));
+ }
/// Allocate a new MachineMemOperand by copying an existing one,
/// replacing only AliasAnalysis information. MachineMemOperands are owned
diff --git a/llvm/include/llvm/CodeGen/MachineMemOperand.h b/llvm/include/llvm/CodeGen/MachineMemOperand.h
index 12c18aaea5b26c..da4ca582cb9e4f 100644
--- a/llvm/include/llvm/CodeGen/MachineMemOperand.h
+++ b/llvm/include/llvm/CodeGen/MachineMemOperand.h
@@ -17,6 +17,7 @@
#include "llvm/ADT/BitmaskEnum.h"
#include "llvm/ADT/PointerUnion.h"
+#include "llvm/Analysis/MemoryLocation.h"
#include "llvm/CodeGen/PseudoSourceValue.h"
#include "llvm/CodeGenTypes/LowLevelType.h"
#include "llvm/IR/DerivedTypes.h"
@@ -186,7 +187,7 @@ class MachineMemOperand {
/// and atomic ordering requirements must also be specified. For cmpxchg
/// atomic operations the atomic ordering requirements when store does not
/// occur must also be specified.
- MachineMemOperand(MachinePointerInfo PtrInfo, Flags flags, uint64_t s,
+ MachineMemOperand(MachinePointerInfo PtrInfo, Flags flags, LocationSize TS,
Align a, const AAMDNodes &AAInfo = AAMDNodes(),
const MDNode *Ranges = nullptr,
SyncScope::ID SSID = SyncScope::System,
@@ -235,13 +236,17 @@ class MachineMemOperand {
LLT getMemoryType() const { return MemoryType; }
/// Return the size in bytes of the memory reference.
- uint64_t getSize() const {
- return MemoryType.isValid() ? MemoryType.getSizeInBytes() : ~UINT64_C(0);
+ LocationSize getSize() const {
+ return MemoryType.isValid()
+ ? LocationSize::precise(MemoryType.getSizeInBytes())
+ : LocationSize::beforeOrAfterPointer();
}
/// Return the size in bits of the memory reference.
- uint64_t getSizeInBits() const {
- return MemoryType.isValid() ? MemoryType.getSizeInBits() : ~UINT64_C(0);
+ LocationSize getSizeInBits() const {
+ return MemoryType.isValid()
+ ? LocationSize::precise(MemoryType.getSizeInBits())
+ : LocationSize::beforeOrAfterPointer();
}
LLT getType() const {
diff --git a/llvm/include/llvm/CodeGen/SelectionDAG.h b/llvm/include/llvm/CodeGen/SelectionDAG.h
index 25e6c525b672a1..4785e93d72d1cc 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAG.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAG.h
@@ -1299,7 +1299,7 @@ class SelectionDAG {
EVT MemVT, MachinePointerInfo PtrInfo, Align Alignment,
MachineMemOperand::Flags Flags = MachineMemOperand::MOLoad |
MachineMemOperand::MOStore,
- uint64_t Size = 0, const AAMDNodes &AAInfo = AAMDNodes());
+ LocationSize Size = 0, const AAMDNodes &AAInfo = AAMDNodes());
inline SDValue getMemIntrinsicNode(
unsigned Opcode, const SDLoc &dl, SDVTList VTList, ArrayRef<SDValue> Ops,
@@ -1307,7 +1307,7 @@ class SelectionDAG {
MaybeAlign Alignment = std::nullopt,
MachineMemOperand::Flags Flags = MachineMemOperand::MOLoad |
MachineMemOperand::MOStore,
- uint64_t Size = 0, const AAMDNodes &AAInfo = AAMDNodes()) {
+ LocationSize Size = 0, const AAMDNodes &AAInfo = AAMDNodes()) {
// Ensure that codegen never sees alignment 0
return getMemIntrinsicNode(Opcode, dl, VTList, Ops, MemVT, PtrInfo,
Alignment.value_or(getEVTAlign(MemVT)), Flags,
diff --git a/llvm/lib/CodeGen/DFAPacketizer.cpp b/llvm/lib/CodeGen/DFAPacketizer.cpp
index 48bb4a07662e10..c16166a1d5e1c5 100644
--- a/llvm/lib/CodeGen/DFAPacketizer.cpp
+++ b/llvm/lib/CodeGen/DFAPacketizer.cpp
@@ -252,12 +252,13 @@ void VLIWPacketizerList::PacketizeMIs(MachineBasicBlock *MBB,
bool VLIWPacketizerList::alias(const MachineMemOperand &Op1,
const MachineMemOperand &Op2,
bool UseTBAA) const {
- if (!Op1.getValue() || !Op2.getValue())
+ if (!Op1.getValue() || !Op2.getValue() || !Op1.getSize().hasValue() ||
+ !Op2.getSize().hasValue())
return true;
int64_t MinOffset = std::min(Op1.getOffset(), Op2.getOffset());
- int64_t Overlapa = Op1.getSize() + Op1.getOffset() - MinOffset;
- int64_t Overlapb = Op2.getSize() + Op2.getOffset() - MinOffset;
+ int64_t Overlapa = Op1.getSize().getValue() + Op1.getOffset() - MinOffset;
+ int64_t Overlapb = Op2.getSize().getValue() + Op2.getOffset() - MinOffset;
AliasResult AAResult =
AA->alias(MemoryLocation(Op1.getValue(), Overlapa,
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index ab055b723dbb1f..79541490ce7412 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -770,12 +770,12 @@ bool CombinerHelper::matchCombineLoadWithAndMask(MachineInstr &MI,
LLT RegTy = MRI.getType(LoadReg);
Register PtrReg = LoadMI->getPointerReg();
unsigned RegSize = RegTy.getSizeInBits();
- uint64_t LoadSizeBits = LoadMI->getMemSizeInBits();
+ LocationSize LoadSizeBits = LoadMI->getMemSizeInBits();
unsigned MaskSizeBits = MaskVal.countr_one();
// The mask may not be larger than the in-memory type, as it might cover sign
// extended bits
- if (MaskSizeBits > LoadSizeBits)
+ if (MaskSizeBits > LoadSizeBits.getValue())
return false;
// If the mask covers the whole destination register, there's nothing to
@@ -795,7 +795,8 @@ bool CombinerHelper::matchCombineLoadWithAndMask(MachineInstr &MI,
// still adjust the opcode to indicate the high bit behavior.
if (LoadMI->isSimple())
MemDesc.MemoryTy = LLT::scalar(MaskSizeBits);
- else if (LoadSizeBits > MaskSizeBits || LoadSizeBits == RegSize)
+ else if (LoadSizeBits.getValue() > MaskSizeBits ||
+ LoadSizeBits.getValue() == RegSize)
return false;
// TODO: Could check if it's legal with the reduced or original memory size.
@@ -860,7 +861,8 @@ bool CombinerHelper::matchSextTruncSextLoad(MachineInstr &MI) {
if (auto *LoadMI = getOpcodeDef<GSExtLoad>(LoadUser, MRI)) {
// If truncating more than the original extended value, abort.
auto LoadSizeBits = LoadMI->getMemSizeInBits();
- if (TruncSrc && MRI.getType(TruncSrc).getSizeInBits() < LoadSizeBits)
+ if (TruncSrc &&
+ MRI.getType(TruncSrc).getSizeInBits() < LoadSizeBits.getValue())
return false;
if (LoadSizeBits == SizeInBits)
return true;
@@ -891,7 +893,7 @@ bool CombinerHelper::matchSextInRegOfLoad(
if (!LoadDef || !MRI.hasOneNonDBGUse(DstReg))
return false;
- uint64_t MemBits = LoadDef->getMemSizeInBits();
+ uint64_t MemBits = LoadDef->getMemSizeInBits().getValue();
// If the sign extend extends from a narrower width than the load's width,
// then we can narrow the load width when we combine to a G_SEXTLOAD.
diff --git a/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp b/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
index 099bf45b2734cb..2e2cc9a95bd95c 100644
--- a/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
@@ -415,7 +415,8 @@ void GISelKnownBits::computeKnownBitsImpl(Register R, KnownBits &Known,
if (DstTy.isVector())
break;
// Everything above the retrieved bits is zero
- Known.Zero.setBitsFrom((*MI.memoperands_begin())->getSizeInBits());
+ Known.Zero.setBitsFrom(
+ (*MI.memoperands_begin())->getSizeInBits().getValue());
break;
}
case TargetOpcode::G_ASHR: {
@@ -666,7 +667,7 @@ unsigned GISelKnownBits::computeNumSignBits(Register R,
// e.g. i16->i32 = '17' bits known.
const MachineMemOperand *MMO = *MI.memoperands_begin();
- return TyBits - MMO->getSizeInBits() + 1;
+ return TyBits - MMO->getSizeInBits().getValue() + 1;
}
case TargetOpcode::G_ZEXTLOAD: {
// FIXME: We need an in-memory type representation.
@@ -675,7 +676,7 @@ unsigned GISelKnownBits::computeNumSignBits(Register R,
// e.g. i16->i32 = '16' bits known.
const MachineMemOperand *MMO = *MI.memoperands_begin();
- return TyBits - MMO->getSizeInBits();
+ return TyBits - MMO->getSizeInBits().getValue();
}
case TargetOpcode::G_TRUNC: {
Register Src = MI.getOperand(1).getReg();
diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index bd3ff7265d51f9..bc062041a564d1 100644
--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -1317,7 +1317,7 @@ LegalizerHelper::LegalizeResult LegalizerHelper::narrowScalar(MachineInstr &MI,
if (DstTy.isVector())
return UnableToLegalize;
- if (8 * LoadMI.getMemSize() != DstTy.getSizeInBits()) {
+ if (8 * LoadMI.getMemSize().getValue() != DstTy.getSizeInBits()) {
Register TmpReg = MRI.createGenericVirtualRegister(NarrowTy);
MIRBuilder.buildLoad(TmpReg, LoadMI.getPointerReg(), LoadMI.getMMO());
MIRBuilder.buildAnyExt(DstReg, TmpReg);
@@ -1335,7 +1335,7 @@ LegalizerHelper::LegalizeResult LegalizerHelper::narrowScalar(MachineInstr &MI,
Register TmpReg = MRI.createGenericVirtualRegister(NarrowTy);
auto &MMO = LoadMI.getMMO();
- unsigned MemSize = MMO.getSizeInBits();
+ unsigned MemSize = MMO.getSizeInBits().getValue();
if (MemSize == NarrowSize) {
MIRBuilder.buildLoad(TmpReg, PtrReg, MMO);
@@ -1368,7 +1368,7 @@ LegalizerHelper::LegalizeResult LegalizerHelper::narrowScalar(MachineInstr &MI,
if (SrcTy.isVector() && LeftoverBits != 0)
return UnableToLegalize;
- if (8 * StoreMI.getMemSize() != SrcTy.getSizeInBits()) {
+ if (8 * StoreMI.getMemSize().getValue() != SrcTy.getSizeInBits()) {
Register TmpReg = MRI.createGenericVirtualRegister(NarrowTy);
MIRBuilder.buildTrunc(TmpReg, SrcReg);
MIRBuilder.buildStore(TmpReg, StoreMI.getPointerReg(), StoreMI.getMMO());
@@ -4456,7 +4456,7 @@ LegalizerHelper::reduceLoadStoreWidth(GLoadStore &LdStMI, unsigned TypeIdx,
LLT ValTy = MRI.getType(ValReg);
// FIXME: Do we need a distinct NarrowMemory legalize action?
- if (ValTy.getSizeInBits() != 8 * LdStMI.getMemSize()) {
+ if (ValTy.getSizeInBits() != 8 * LdStMI.getMemSize().getValue()) {
LLVM_DEBUG(dbgs() << "Can't narrow extload/truncstore\n");
return UnableToLegalize;
}
diff --git a/llvm/lib/CodeGen/GlobalISel/LoadStoreOpt.cpp b/llvm/lib/CodeGen/GlobalISel/LoadStoreOpt.cpp
index b5c9d3e912cc20..9fc8ecd60b03ff 100644
--- a/llvm/lib/CodeGen/GlobalISel/LoadStoreOpt.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LoadStoreOpt.cpp
@@ -117,12 +117,8 @@ bool GISelAddressing::aliasIsKnownForLoadStore(const MachineInstr &MI1,
if (!BasePtr0.BaseReg.isValid() || !BasePtr1.BaseReg.isValid())
return false;
- LocationSize Size1 = LdSt1->getMemSize() != MemoryLocation::UnknownSize
- ? LdSt1->getMemSize()
- : LocationSize::beforeOrAfterPointer();
- LocationSize Size2 = LdSt2->getMemSize() != MemoryLocation::UnknownSize
- ? LdSt2->getMemSize()
- : LocationSize::beforeOrAfterPointer();
+ LocationSize Size1 = LdSt1->getMemSize();
+ LocationSize Size2 = LdSt2->getMemSize();
int64_t PtrDiff;
if (BasePtr0.BaseReg == BasePtr1.BaseReg) {
@@ -214,14 +210,9 @@ bool GISelAddressing::instMayAlias(const MachineInstr &MI,
Offset = 0;
}
- TypeSize Size = LS->getMMO().getMemoryType().getSizeInBytes();
- return {LS->isVolatile(),
- LS->isAtomic(),
- BaseReg,
- Offset /*base offset*/,
- Size.isScalable() ? LocationSize::beforeOrAfterPointer()
- : LocationSize::precise(Size),
- &LS->getMMO()};
+ LocationSize Size = LS->getMMO().getSize();
+ return {LS->isVolatile(), LS->isAtomic(), BaseReg,
+ Offset /*base offset*/, Size, &LS->getMMO()};
}
// FIXME: support recognizing lifetime instructions.
// Default.
diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
index cfc8c28b99e562..481d9e341da377 100644
--- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
+++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
@@ -1356,10 +1356,11 @@ InstrRefBasedLDV::findLocationForMemOperand(const MachineInstr &MI) {
// from the stack at some point. Happily the memory operand will tell us
// the size written to the stack.
auto *MemOperand = *MI.memoperands_begin();
- unsigned SizeInBits = MemOperand->getSizeInBits();
+ LocationSize SizeInBits = MemOperand->getSizeInBits();
+ assert(SizeInBits.hasValue() && "Expected to find a valid size!");
// Find that position in the stack indexes we're tracking.
- auto IdxIt = MTracker->StackSlotIdxes.find({SizeInB...
[truncated]
|
arsenm
reviewed
Mar 12, 2024
| @@ -647,15 +647,15 @@ bool GIMatchTableExecutor::executeMatchTable( | |||
|
|
|||
| unsigned Size = MRI.getType(MO.getReg()).getSizeInBits(); | |||
There was a problem hiding this comment.
should Size not be unsigned here?
There was a problem hiding this comment.
Do you mean it should be a TypeSize? I think this is a bit of code likely to need to change as GISel learns about scalable vectors (with the >=/<=). I think most of GISel should assume the memory sizes cannot be unknown for these nodes.
| return getMachineMemOperand( | ||
| MMO, Offset, Size == ~UINT64_C(0) ? LLT() : LLT::scalar(8 * Size)); | ||
| MMO, Offset, | ||
| !Size.hasValue() || Size.isScalable() |
There was a problem hiding this comment.
Can this just assert Size.hasValue()? I would assume if you have the unknown case you would have been using the LLT constructor in the first place
There was a problem hiding this comment.
I think quite a lot of things will construct an MMO with an unknown LocationSize, either directly or from the size in another MMO that was already unknown.
~UINT64_C(0) and MemoryLocation::UnknownSize was used a lot in the past, they have all been changed to LocationSize::beforeOrAfterPointer() as the unknown Size.
arsenm
approved these changes
Mar 14, 2024
This is part of llvm#70452 that changes the type used for the external interface of MMO to LocationSize as opposed to uint64_t. This means the constructors take LocationSize, and convert ~UINT64_C(0) to LocationSize::beforeOrAfter(). The getSize methods return a LocationSize. This allows us to be more precise with unknown sizes, not accidentally treating them as unsigned values, and in the future should allow us to add proper scalable vector support but none of that is included in this patch. It should mostly be an NFC. Global ISel is still expected to use the underlying LLT as it needs, and are not expected to see unknown sizes for generic operations. Most of the changes are hopefully fairly mechanical.
davemgreen
force-pushed
the
gh-scalableaa-locationsize
branch
from
e725aa7
to
bd6fcd7
Compare
|
Thanks. I've ran some testing, which all checked out, but this has a chance of hitting asserts in cases that were previously just converted to int. Let me know if this runs into any problems. |
davemgreen
added a commit
that referenced
this pull request
Mar 18, 2024
As an extension to #84751, this adds some extra uses of beforeOrAfterPointer() instead of UnknownSize.
chencha3
pushed a commit
to chencha3/llvm-project
that referenced
this pull request
Mar 23, 2024
As an extension to llvm#84751, this adds some extra uses of beforeOrAfterPointer() instead of UnknownSize.
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.
This is part of #70452 that changes the type used for the external interface of MMO to LocationSize as opposed to uint64_t. This means the constructors take LocationSize, and convert ~UINT64_C(0) to LocationSize::beforeOrAfter(). The getSize methods return a LocationSize.
This allows us to be more precise with unknown sizes, not accidentally treating them as unsigned values, and in the future should allow us to add proper scalable vector support but none of that is included in this patch. It should mostly be an NFC.
Global ISel is still expected to use the underlying LLT as it needs, and are not expected to see unknown sizes for generic operations. Most of the changes are hopefully fairly mechanical, adding a lot of getValue() calls and protecting them with hasValue() where needed.