[GlobalISel][NFC] Fix LLT Propagation #119587
Conversation
|
@llvm/pr-subscribers-backend-amdgpu Author: Tim Gymnich (tgymnich) ChangesRetain LLT type information by creating new LLTs from the original LLT instead of only using the original scalar size. This PR prepares for the LLT FPInfo RFC where LLTs will carry additional floating point type information in addition to the scalar size. Full diff: https://github.com/llvm/llvm-project/pull/119587.diff 3 Files Affected:
diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index f56be39838ba7d..45ba746780ef48 100644
--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -5347,9 +5347,8 @@ LegalizerHelper::fewerElementsBitcast(MachineInstr &MI, unsigned int TypeIdx,
auto [DstReg, DstTy, SrcReg, SrcTy] = MI.getFirst2RegLLTs();
- unsigned SrcScalSize = SrcTy.getScalarSizeInBits();
- LLT SrcNarrowTy =
- LLT::fixed_vector(NarrowTy.getSizeInBits() / SrcScalSize, SrcScalSize);
+ unsigned NewElemCount = NarrowTy.getSizeInBits() / SrcTy.getScalarSizeInBits();
+ LLT SrcNarrowTy = LLT::fixed_vector(NewElemCount, SrcTy.getElementType());
// Split the Src and Dst Reg into smaller registers
SmallVector<Register> SrcVRegs, BitcastVRegs;
diff --git a/llvm/lib/CodeGen/GlobalISel/Utils.cpp b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
index 45807a6818ee5e..01fe9e7591954b 100644
--- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
@@ -526,7 +526,7 @@ bool llvm::extractParts(Register Reg, LLT RegTy, LLT MainTy, LLT &LeftoverTy,
RegTy.getScalarSizeInBits() == MainTy.getScalarSizeInBits() &&
LeftoverNumElts > 1) {
LeftoverTy =
- LLT::fixed_vector(LeftoverNumElts, RegTy.getScalarSizeInBits());
+ LLT::fixed_vector(LeftoverNumElts, RegTy.getElementType());
// Unmerge the SrcReg to LeftoverTy vectors
SmallVector<Register, 4> UnmergeValues;
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
index 2e66f7525b9ccf..730fb3260bb36b 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
@@ -154,8 +154,7 @@ static LegalizeMutation moreElementsToNextExistingRegClass(unsigned TypeIdx) {
if (SIRegisterInfo::getSGPRClassForBitWidth(NewNumElts * EltSize))
break;
}
-
- return std::pair(TypeIdx, LLT::fixed_vector(NewNumElts, EltSize));
+ return std::pair(TypeIdx, LLT::fixed_vector(NewNumElts, Ty.getElementType()));
};
}
|
|
@llvm/pr-subscribers-llvm-globalisel Author: Tim Gymnich (tgymnich) ChangesRetain LLT type information by creating new LLTs from the original LLT instead of only using the original scalar size. This PR prepares for the LLT FPInfo RFC where LLTs will carry additional floating point type information in addition to the scalar size. Full diff: https://github.com/llvm/llvm-project/pull/119587.diff 3 Files Affected:
diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index f56be39838ba7d..45ba746780ef48 100644
--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -5347,9 +5347,8 @@ LegalizerHelper::fewerElementsBitcast(MachineInstr &MI, unsigned int TypeIdx,
auto [DstReg, DstTy, SrcReg, SrcTy] = MI.getFirst2RegLLTs();
- unsigned SrcScalSize = SrcTy.getScalarSizeInBits();
- LLT SrcNarrowTy =
- LLT::fixed_vector(NarrowTy.getSizeInBits() / SrcScalSize, SrcScalSize);
+ unsigned NewElemCount = NarrowTy.getSizeInBits() / SrcTy.getScalarSizeInBits();
+ LLT SrcNarrowTy = LLT::fixed_vector(NewElemCount, SrcTy.getElementType());
// Split the Src and Dst Reg into smaller registers
SmallVector<Register> SrcVRegs, BitcastVRegs;
diff --git a/llvm/lib/CodeGen/GlobalISel/Utils.cpp b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
index 45807a6818ee5e..01fe9e7591954b 100644
--- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
@@ -526,7 +526,7 @@ bool llvm::extractParts(Register Reg, LLT RegTy, LLT MainTy, LLT &LeftoverTy,
RegTy.getScalarSizeInBits() == MainTy.getScalarSizeInBits() &&
LeftoverNumElts > 1) {
LeftoverTy =
- LLT::fixed_vector(LeftoverNumElts, RegTy.getScalarSizeInBits());
+ LLT::fixed_vector(LeftoverNumElts, RegTy.getElementType());
// Unmerge the SrcReg to LeftoverTy vectors
SmallVector<Register, 4> UnmergeValues;
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
index 2e66f7525b9ccf..730fb3260bb36b 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
@@ -154,8 +154,7 @@ static LegalizeMutation moreElementsToNextExistingRegClass(unsigned TypeIdx) {
if (SIRegisterInfo::getSGPRClassForBitWidth(NewNumElts * EltSize))
break;
}
-
- return std::pair(TypeIdx, LLT::fixed_vector(NewNumElts, EltSize));
+ return std::pair(TypeIdx, LLT::fixed_vector(NewNumElts, Ty.getElementType()));
};
}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Retain LLT type information by creating new LLTs from the original LLT instead of only using the original scalar size.
tgymnich
force-pushed
the
fix-llt-propagation
branch
from
c875417 to
71509ea
Compare
|
A tracking issue would be nice, but ... Should we long-term deprecate those vector constructors that take bits? |
@tschuett Yes, definitely. That would be one of the goals of the RFC. I'll create a follow up PR containing these changes. |
|
I'm wondering if we should add a new API so that it's convenient to create a new LLT based on the existing vector LLT. Instead of having to manually call |
Ah, we already have that overload for the scalar variant. We can deal with this in another change. |
| unsigned NewElemCount = | ||
| NarrowTy.getSizeInBits() / SrcTy.getScalarSizeInBits(); | ||
| LLT SrcNarrowTy = LLT::fixed_vector(NewElemCount, SrcTy.getElementType()); |
There was a problem hiding this comment.
I think this could use LLT::divide
There was a problem hiding this comment.
LLT::divide operates on element counts not on bit sizes.
|
Tracking issue: #119667 |
tgymnich
changed the title
|
Feel free to merge this for me since I do not have commit access. In the meanwhile I already started drafting a PR that removes initialization with scalar sizes: #119725 |
Sure, just a tip on GitHub workflow though: it's recommended you don't merge into your PR branch from main. If you need to update you should do a rebase and then force push the new branch. In most cases however rebasing isn't done either unless it's been a long time since it can also mess with the review comment UI a bit. |
Retain LLT type information by creating new LLTs from the original LLT instead of only using the original scalar size.
This PR prepares for the LLT FPInfo RFC where LLTs will carry additional floating point type information in addition to the scalar size.