[TTI] Use TypeSize in isLoadFromStackSlot and isStoreToStackSlot [nfc] #132244
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
Motivation is supporting scalable spills and reloads, e.g. in llvm#120524. Looking at this API, I'm suspicious that the access size should just be coming from the memory operand on the load or store, but we don't appear to be consistently setting that up. That's a larger change so I may or may not bother pursuing that.
|
@llvm/pr-subscribers-backend-risc-v @llvm/pr-subscribers-backend-x86 Author: Philip Reames (preames) ChangesMotivation is supporting scalable spills and reloads, e.g. in #120524. Looking at this API, I'm suspicious that the access size should just be coming from the memory operand on the load or store, but we don't appear to be consistently setting that up. That's a larger change so I may or may not bother pursuing that. Full diff: https://github.com/llvm/llvm-project/pull/132244.diff 6 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index 8a69f28869628..b603650cae459 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -31,6 +31,7 @@
#include "llvm/MC/MCInstrInfo.h"
#include "llvm/Support/BranchProbability.h"
#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/TypeSize.h"
#include <array>
#include <cassert>
#include <cstddef>
@@ -293,8 +294,8 @@ class TargetInstrInfo : public MCInstrInfo {
/// what the load does.
virtual Register isLoadFromStackSlot(const MachineInstr &MI,
int &FrameIndex,
- unsigned &MemBytes) const {
- MemBytes = 0;
+ TypeSize &MemBytes) const {
+ MemBytes = TypeSize::getFixed(0);
return isLoadFromStackSlot(MI, FrameIndex);
}
@@ -331,8 +332,8 @@ class TargetInstrInfo : public MCInstrInfo {
/// what the store does.
virtual Register isStoreToStackSlot(const MachineInstr &MI,
int &FrameIndex,
- unsigned &MemBytes) const {
- MemBytes = 0;
+ TypeSize &MemBytes) const {
+ MemBytes = TypeSize::getFixed(0);
return isStoreToStackSlot(MI, FrameIndex);
}
diff --git a/llvm/lib/CodeGen/StackSlotColoring.cpp b/llvm/lib/CodeGen/StackSlotColoring.cpp
index 22c5c2e080b05..5dfacce016a16 100644
--- a/llvm/lib/CodeGen/StackSlotColoring.cpp
+++ b/llvm/lib/CodeGen/StackSlotColoring.cpp
@@ -484,8 +484,8 @@ bool StackSlotColoring::RemoveDeadStores(MachineBasicBlock* MBB) {
Register LoadReg;
Register StoreReg;
- unsigned LoadSize = 0;
- unsigned StoreSize = 0;
+ TypeSize LoadSize = TypeSize::getFixed(0);
+ TypeSize StoreSize = TypeSize::getFixed(0);
if (!(LoadReg = TII->isLoadFromStackSlot(*I, FirstSS, LoadSize)))
continue;
// Skip the ...pseudo debugging... instructions between a load and store.
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 3a04344f8237f..571c3689e6f76 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -95,35 +95,35 @@ MCInst RISCVInstrInfo::getNop() const {
Register RISCVInstrInfo::isLoadFromStackSlot(const MachineInstr &MI,
int &FrameIndex) const {
- unsigned Dummy;
+ TypeSize Dummy = TypeSize::getFixed(0);
return isLoadFromStackSlot(MI, FrameIndex, Dummy);
}
Register RISCVInstrInfo::isLoadFromStackSlot(const MachineInstr &MI,
int &FrameIndex,
- unsigned &MemBytes) const {
+ TypeSize &MemBytes) const {
switch (MI.getOpcode()) {
default:
return 0;
case RISCV::LB:
case RISCV::LBU:
- MemBytes = 1;
+ MemBytes = TypeSize::getFixed(1);
break;
case RISCV::LH:
case RISCV::LH_INX:
case RISCV::LHU:
case RISCV::FLH:
- MemBytes = 2;
+ MemBytes = TypeSize::getFixed(2);
break;
case RISCV::LW:
case RISCV::LW_INX:
case RISCV::FLW:
case RISCV::LWU:
- MemBytes = 4;
+ MemBytes = TypeSize::getFixed(4);
break;
case RISCV::LD:
case RISCV::FLD:
- MemBytes = 8;
+ MemBytes = TypeSize::getFixed(8);
break;
}
@@ -138,32 +138,32 @@ Register RISCVInstrInfo::isLoadFromStackSlot(const MachineInstr &MI,
Register RISCVInstrInfo::isStoreToStackSlot(const MachineInstr &MI,
int &FrameIndex) const {
- unsigned Dummy;
+ TypeSize Dummy = TypeSize::getFixed(0);
return isStoreToStackSlot(MI, FrameIndex, Dummy);
}
Register RISCVInstrInfo::isStoreToStackSlot(const MachineInstr &MI,
int &FrameIndex,
- unsigned &MemBytes) const {
+ TypeSize &MemBytes) const {
switch (MI.getOpcode()) {
default:
return 0;
case RISCV::SB:
- MemBytes = 1;
+ MemBytes = TypeSize::getFixed(1);
break;
case RISCV::SH:
case RISCV::SH_INX:
case RISCV::FSH:
- MemBytes = 2;
+ MemBytes = TypeSize::getFixed(2);
break;
case RISCV::SW:
case RISCV::SW_INX:
case RISCV::FSW:
- MemBytes = 4;
+ MemBytes = TypeSize::getFixed(4);
break;
case RISCV::SD:
case RISCV::FSD:
- MemBytes = 8;
+ MemBytes = TypeSize::getFixed(8);
break;
}
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.h b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
index d68bd58885873..fcf296fcba74b 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.h
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
@@ -70,11 +70,11 @@ class RISCVInstrInfo : public RISCVGenInstrInfo {
Register isLoadFromStackSlot(const MachineInstr &MI,
int &FrameIndex) const override;
Register isLoadFromStackSlot(const MachineInstr &MI, int &FrameIndex,
- unsigned &MemBytes) const override;
+ TypeSize &MemBytes) const override;
Register isStoreToStackSlot(const MachineInstr &MI,
int &FrameIndex) const override;
Register isStoreToStackSlot(const MachineInstr &MI, int &FrameIndex,
- unsigned &MemBytes) const override;
+ TypeSize &MemBytes) const override;
bool isReallyTriviallyReMaterializable(const MachineInstr &MI) const override;
diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp
index 5c65171dd83b0..3cb7c0b6518f1 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.cpp
+++ b/llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -486,21 +486,21 @@ bool X86InstrInfo::isFrameOperand(const MachineInstr &MI, unsigned int Op,
return false;
}
-static bool isFrameLoadOpcode(int Opcode, unsigned &MemBytes) {
+static bool isFrameLoadOpcode(int Opcode, TypeSize &MemBytes) {
switch (Opcode) {
default:
return false;
case X86::MOV8rm:
case X86::KMOVBkm:
case X86::KMOVBkm_EVEX:
- MemBytes = 1;
+ MemBytes = TypeSize::getFixed(1);
return true;
case X86::MOV16rm:
case X86::KMOVWkm:
case X86::KMOVWkm_EVEX:
case X86::VMOVSHZrm:
case X86::VMOVSHZrm_alt:
- MemBytes = 2;
+ MemBytes = TypeSize::getFixed(2);
return true;
case X86::MOV32rm:
case X86::MOVSSrm:
@@ -511,7 +511,7 @@ static bool isFrameLoadOpcode(int Opcode, unsigned &MemBytes) {
case X86::VMOVSSZrm_alt:
case X86::KMOVDkm:
case X86::KMOVDkm_EVEX:
- MemBytes = 4;
+ MemBytes = TypeSize::getFixed(4);
return true;
case X86::MOV64rm:
case X86::LD_Fp64m:
@@ -525,7 +525,7 @@ static bool isFrameLoadOpcode(int Opcode, unsigned &MemBytes) {
case X86::MMX_MOVQ64rm:
case X86::KMOVQkm:
case X86::KMOVQkm_EVEX:
- MemBytes = 8;
+ MemBytes = TypeSize::getFixed(8);
return true;
case X86::MOVAPSrm:
case X86::MOVUPSrm:
@@ -551,7 +551,7 @@ static bool isFrameLoadOpcode(int Opcode, unsigned &MemBytes) {
case X86::VMOVDQU32Z128rm:
case X86::VMOVDQA64Z128rm:
case X86::VMOVDQU64Z128rm:
- MemBytes = 16;
+ MemBytes = TypeSize::getFixed(16);
return true;
case X86::VMOVAPSYrm:
case X86::VMOVUPSYrm:
@@ -571,7 +571,7 @@ static bool isFrameLoadOpcode(int Opcode, unsigned &MemBytes) {
case X86::VMOVDQU32Z256rm:
case X86::VMOVDQA64Z256rm:
case X86::VMOVDQU64Z256rm:
- MemBytes = 32;
+ MemBytes = TypeSize::getFixed(32);
return true;
case X86::VMOVAPSZrm:
case X86::VMOVUPSZrm:
@@ -583,25 +583,25 @@ static bool isFrameLoadOpcode(int Opcode, unsigned &MemBytes) {
case X86::VMOVDQU32Zrm:
case X86::VMOVDQA64Zrm:
case X86::VMOVDQU64Zrm:
- MemBytes = 64;
+ MemBytes = TypeSize::getFixed(64);
return true;
}
}
-static bool isFrameStoreOpcode(int Opcode, unsigned &MemBytes) {
+static bool isFrameStoreOpcode(int Opcode, TypeSize &MemBytes) {
switch (Opcode) {
default:
return false;
case X86::MOV8mr:
case X86::KMOVBmk:
case X86::KMOVBmk_EVEX:
- MemBytes = 1;
+ MemBytes = TypeSize::getFixed(1);
return true;
case X86::MOV16mr:
case X86::KMOVWmk:
case X86::KMOVWmk_EVEX:
case X86::VMOVSHZmr:
- MemBytes = 2;
+ MemBytes = TypeSize::getFixed(2);
return true;
case X86::MOV32mr:
case X86::MOVSSmr:
@@ -609,7 +609,7 @@ static bool isFrameStoreOpcode(int Opcode, unsigned &MemBytes) {
case X86::VMOVSSZmr:
case X86::KMOVDmk:
case X86::KMOVDmk_EVEX:
- MemBytes = 4;
+ MemBytes = TypeSize::getFixed(4);
return true;
case X86::MOV64mr:
case X86::ST_FpP64m:
@@ -621,7 +621,7 @@ static bool isFrameStoreOpcode(int Opcode, unsigned &MemBytes) {
case X86::MMX_MOVNTQmr:
case X86::KMOVQmk:
case X86::KMOVQmk_EVEX:
- MemBytes = 8;
+ MemBytes = TypeSize::getFixed(8);
return true;
case X86::MOVAPSmr:
case X86::MOVUPSmr:
@@ -647,7 +647,7 @@ static bool isFrameStoreOpcode(int Opcode, unsigned &MemBytes) {
case X86::VMOVDQU64Z128mr:
case X86::VMOVDQU8Z128mr:
case X86::VMOVDQU16Z128mr:
- MemBytes = 16;
+ MemBytes = TypeSize::getFixed(16);
return true;
case X86::VMOVUPSYmr:
case X86::VMOVAPSYmr:
@@ -667,7 +667,7 @@ static bool isFrameStoreOpcode(int Opcode, unsigned &MemBytes) {
case X86::VMOVDQU32Z256mr:
case X86::VMOVDQA64Z256mr:
case X86::VMOVDQU64Z256mr:
- MemBytes = 32;
+ MemBytes = TypeSize::getFixed(32);
return true;
case X86::VMOVUPSZmr:
case X86::VMOVAPSZmr:
@@ -679,7 +679,7 @@ static bool isFrameStoreOpcode(int Opcode, unsigned &MemBytes) {
case X86::VMOVDQU32Zmr:
case X86::VMOVDQA64Zmr:
case X86::VMOVDQU64Zmr:
- MemBytes = 64;
+ MemBytes = TypeSize::getFixed(64);
return true;
}
return false;
@@ -687,13 +687,13 @@ static bool isFrameStoreOpcode(int Opcode, unsigned &MemBytes) {
Register X86InstrInfo::isLoadFromStackSlot(const MachineInstr &MI,
int &FrameIndex) const {
- unsigned Dummy;
+ TypeSize Dummy = TypeSize::getFixed(0);
return X86InstrInfo::isLoadFromStackSlot(MI, FrameIndex, Dummy);
}
Register X86InstrInfo::isLoadFromStackSlot(const MachineInstr &MI,
int &FrameIndex,
- unsigned &MemBytes) const {
+ TypeSize &MemBytes) const {
if (isFrameLoadOpcode(MI.getOpcode(), MemBytes))
if (MI.getOperand(0).getSubReg() == 0 && isFrameOperand(MI, 1, FrameIndex))
return MI.getOperand(0).getReg();
@@ -702,7 +702,7 @@ Register X86InstrInfo::isLoadFromStackSlot(const MachineInstr &MI,
Register X86InstrInfo::isLoadFromStackSlotPostFE(const MachineInstr &MI,
int &FrameIndex) const {
- unsigned Dummy;
+ TypeSize Dummy = TypeSize::getFixed(0);
if (isFrameLoadOpcode(MI.getOpcode(), Dummy)) {
if (Register Reg = isLoadFromStackSlot(MI, FrameIndex))
return Reg;
@@ -720,13 +720,13 @@ Register X86InstrInfo::isLoadFromStackSlotPostFE(const MachineInstr &MI,
Register X86InstrInfo::isStoreToStackSlot(const MachineInstr &MI,
int &FrameIndex) const {
- unsigned Dummy;
+ TypeSize Dummy = TypeSize::getFixed(0);
return X86InstrInfo::isStoreToStackSlot(MI, FrameIndex, Dummy);
}
Register X86InstrInfo::isStoreToStackSlot(const MachineInstr &MI,
int &FrameIndex,
- unsigned &MemBytes) const {
+ TypeSize &MemBytes) const {
if (isFrameStoreOpcode(MI.getOpcode(), MemBytes))
if (MI.getOperand(X86::AddrNumOperands).getSubReg() == 0 &&
isFrameOperand(MI, 0, FrameIndex))
@@ -736,7 +736,7 @@ Register X86InstrInfo::isStoreToStackSlot(const MachineInstr &MI,
Register X86InstrInfo::isStoreToStackSlotPostFE(const MachineInstr &MI,
int &FrameIndex) const {
- unsigned Dummy;
+ TypeSize Dummy = TypeSize::getFixed(0);
if (isFrameStoreOpcode(MI.getOpcode(), Dummy)) {
if (Register Reg = isStoreToStackSlot(MI, FrameIndex))
return Reg;
diff --git a/llvm/lib/Target/X86/X86InstrInfo.h b/llvm/lib/Target/X86/X86InstrInfo.h
index 403964a305ba5..9e0463cba4579 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.h
+++ b/llvm/lib/Target/X86/X86InstrInfo.h
@@ -276,7 +276,7 @@ class X86InstrInfo final : public X86GenInstrInfo {
int &FrameIndex) const override;
Register isLoadFromStackSlot(const MachineInstr &MI,
int &FrameIndex,
- unsigned &MemBytes) const override;
+ TypeSize &MemBytes) const override;
/// isLoadFromStackSlotPostFE - Check for post-frame ptr elimination
/// stack locations as well. This uses a heuristic so it isn't
/// reliable for correctness.
@@ -287,7 +287,7 @@ class X86InstrInfo final : public X86GenInstrInfo {
int &FrameIndex) const override;
Register isStoreToStackSlot(const MachineInstr &MI,
int &FrameIndex,
- unsigned &MemBytes) const override;
+ TypeSize &MemBytes) const override;
/// isStoreToStackSlotPostFE - Check for post-frame ptr elimination
/// stack locations as well. This uses a heuristic so it isn't
/// reliable for correctness.
|
You can test this locally with the following command:git-clang-format --diff 2a53358eea7f8da9ffb92d375976b3f78ce5f212 ff65719c2c4da303f2ea714a7bebf28afef20d14 --extensions h,cpp -- llvm/include/llvm/CodeGen/TargetInstrInfo.h llvm/lib/CodeGen/StackSlotColoring.cpp llvm/lib/Target/RISCV/RISCVInstrInfo.cpp llvm/lib/Target/RISCV/RISCVInstrInfo.h llvm/lib/Target/X86/X86InstrInfo.cpp llvm/lib/Target/X86/X86InstrInfo.hView the diff from clang-format here.diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index 8f2792c1cb..b2900f7415 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -292,8 +292,7 @@ public:
/// bytes loaded from the stack. This must be implemented if a backend
/// supports partial stack slot spills/loads to further disambiguate
/// what the load does.
- virtual Register isLoadFromStackSlot(const MachineInstr &MI,
- int &FrameIndex,
+ virtual Register isLoadFromStackSlot(const MachineInstr &MI, int &FrameIndex,
TypeSize &MemBytes) const {
MemBytes = TypeSize::getZero();
return isLoadFromStackSlot(MI, FrameIndex);
@@ -330,8 +329,7 @@ public:
/// bytes stored to the stack. This must be implemented if a backend
/// supports partial stack slot spills/loads to further disambiguate
/// what the store does.
- virtual Register isStoreToStackSlot(const MachineInstr &MI,
- int &FrameIndex,
+ virtual Register isStoreToStackSlot(const MachineInstr &MI, int &FrameIndex,
TypeSize &MemBytes) const {
MemBytes = TypeSize::getZero();
return isStoreToStackSlot(MI, FrameIndex);
diff --git a/llvm/lib/Target/X86/X86InstrInfo.h b/llvm/lib/Target/X86/X86InstrInfo.h
index 9e0463cba4..767bd7cbdd 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.h
+++ b/llvm/lib/Target/X86/X86InstrInfo.h
@@ -274,8 +274,7 @@ public:
Register isLoadFromStackSlot(const MachineInstr &MI,
int &FrameIndex) const override;
- Register isLoadFromStackSlot(const MachineInstr &MI,
- int &FrameIndex,
+ Register isLoadFromStackSlot(const MachineInstr &MI, int &FrameIndex,
TypeSize &MemBytes) const override;
/// isLoadFromStackSlotPostFE - Check for post-frame ptr elimination
/// stack locations as well. This uses a heuristic so it isn't
@@ -285,8 +284,7 @@ public:
Register isStoreToStackSlot(const MachineInstr &MI,
int &FrameIndex) const override;
- Register isStoreToStackSlot(const MachineInstr &MI,
- int &FrameIndex,
+ Register isStoreToStackSlot(const MachineInstr &MI, int &FrameIndex,
TypeSize &MemBytes) const override;
/// isStoreToStackSlotPostFE - Check for post-frame ptr elimination
/// stack locations as well. This uses a heuristic so it isn't
|
topperc
reviewed
Mar 20, 2025
mshockwave
reviewed
Mar 20, 2025
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.
Motivation is supporting scalable spills and reloads, e.g. in #120524.
Looking at this API, I'm suspicious that the access size should just be coming from the memory operand on the load or store, but we don't appear to be consistently setting that up. That's a larger change so I may or may not bother pursuing that.