Skip to content

Commit 2ed8053

Browse files
committed
Revert "[AArch64][GlobalISel] Don't contract cross-bank copies into truncating stores."
This reverts commit 67bf3ac. The reason is that this change is now superseded by 04fb9b7 which fixes the underlying problem in the selector. Now it's fine to generate truncating FP stores since the selector code will just generate subreg copies to handle them.
1 parent c5735fa commit 2ed8053

File tree

2 files changed

+10
-36
lines changed

2 files changed

+10
-36
lines changed

llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp

+8-34
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ class AArch64InstructionSelector : public InstructionSelector {
105105
bool earlySelectSHL(MachineInstr &I, MachineRegisterInfo &MRI);
106106

107107
/// Eliminate same-sized cross-bank copies into stores before selectImpl().
108-
bool contractCrossBankCopyIntoStore(GStore &I,
108+
bool contractCrossBankCopyIntoStore(MachineInstr &I,
109109
MachineRegisterInfo &MRI);
110110

111111
bool convertPtrAddToAdd(MachineInstr &I, MachineRegisterInfo &MRI);
@@ -1943,9 +1943,8 @@ bool AArch64InstructionSelector::preISelLower(MachineInstr &I) {
19431943
return true;
19441944
}
19451945
case TargetOpcode::G_STORE: {
1946-
auto &StoreMI = cast<GStore>(I);
1947-
bool Changed = contractCrossBankCopyIntoStore(StoreMI, MRI);
1948-
MachineOperand &SrcOp = StoreMI.getOperand(0);
1946+
bool Changed = contractCrossBankCopyIntoStore(I, MRI);
1947+
MachineOperand &SrcOp = I.getOperand(0);
19491948
if (MRI.getType(SrcOp.getReg()).isPointer()) {
19501949
// Allow matching with imported patterns for stores of pointers. Unlike
19511950
// G_LOAD/G_PTR_ADD, we may not have selected all users. So, emit a copy
@@ -1956,28 +1955,6 @@ bool AArch64InstructionSelector::preISelLower(MachineInstr &I) {
19561955
RBI.constrainGenericRegister(NewSrc, AArch64::GPR64RegClass, MRI);
19571956
Changed = true;
19581957
}
1959-
#if 0
1960-
// Now look for truncating stores to the FPR bank. We don't support these,
1961-
// but since truncating store formation happens before RBS, we can only
1962-
// split them up again here. We don't want to assign truncstores to GPR only
1963-
// since that would have a perf impact due to extra moves.
1964-
LLT SrcTy = MRI.getType(SrcReg);
1965-
if (RBI.getRegBank(SrcReg, MRI, TRI)->getID() == AArch64::FPRRegBankID) {
1966-
if (SrcTy.isScalar() &&
1967-
SrcTy.getSizeInBits() > StoreMI.getMemSizeInBits()) {
1968-
// Generate an explicit truncate and make this into a non-truncating
1969-
// store.
1970-
auto Trunc =
1971-
MIB.buildTrunc(LLT::scalar(StoreMI.getMemSizeInBits()), SrcReg);
1972-
MRI.setRegBank(Trunc.getReg(0), RBI.getRegBank(AArch64::FPRRegBankID));
1973-
if (!select(*Trunc)) {
1974-
return false;
1975-
}
1976-
SrcOp.setReg(Trunc.getReg(0));
1977-
return true;
1978-
}
1979-
}
1980-
#endif
19811958
return Changed;
19821959
}
19831960
case TargetOpcode::G_PTR_ADD:
@@ -2113,7 +2090,8 @@ bool AArch64InstructionSelector::earlySelectSHL(MachineInstr &I,
21132090
}
21142091

21152092
bool AArch64InstructionSelector::contractCrossBankCopyIntoStore(
2116-
GStore &StoreMI, MachineRegisterInfo &MRI) {
2093+
MachineInstr &I, MachineRegisterInfo &MRI) {
2094+
assert(I.getOpcode() == TargetOpcode::G_STORE && "Expected G_STORE");
21172095
// If we're storing a scalar, it doesn't matter what register bank that
21182096
// scalar is on. All that matters is the size.
21192097
//
@@ -2128,11 +2106,11 @@ bool AArch64InstructionSelector::contractCrossBankCopyIntoStore(
21282106
// G_STORE %x:gpr(s32)
21292107
//
21302108
// And then continue the selection process normally.
2131-
Register DefDstReg = getSrcRegIgnoringCopies(StoreMI.getValueReg(), MRI);
2109+
Register DefDstReg = getSrcRegIgnoringCopies(I.getOperand(0).getReg(), MRI);
21322110
if (!DefDstReg.isValid())
21332111
return false;
21342112
LLT DefDstTy = MRI.getType(DefDstReg);
2135-
Register StoreSrcReg = StoreMI.getValueReg();
2113+
Register StoreSrcReg = I.getOperand(0).getReg();
21362114
LLT StoreSrcTy = MRI.getType(StoreSrcReg);
21372115

21382116
// If we get something strange like a physical register, then we shouldn't
@@ -2144,16 +2122,12 @@ bool AArch64InstructionSelector::contractCrossBankCopyIntoStore(
21442122
if (DefDstTy.getSizeInBits() != StoreSrcTy.getSizeInBits())
21452123
return false;
21462124

2147-
// Is this store a truncating one?
2148-
if (StoreSrcTy.getSizeInBits() != StoreMI.getMemSizeInBits())
2149-
return false;
2150-
21512125
if (RBI.getRegBank(StoreSrcReg, MRI, TRI) ==
21522126
RBI.getRegBank(DefDstReg, MRI, TRI))
21532127
return false;
21542128

21552129
// We have a cross-bank copy, which is entering a store. Let's fold it.
2156-
StoreMI.getOperand(0).setReg(DefDstReg);
2130+
I.getOperand(0).setReg(DefDstReg);
21572131
return true;
21582132
}
21592133

llvm/test/CodeGen/AArch64/GlobalISel/contract-store.mir

+2-2
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,8 @@ body: |
131131
; CHECK-LABEL: name: contract_s16_truncstore
132132
; CHECK: [[COPY:%[0-9]+]]:gpr64sp = COPY $x0
133133
; CHECK: [[COPY1:%[0-9]+]]:fpr32 = COPY $s1
134-
; CHECK: [[COPY2:%[0-9]+]]:gpr32 = COPY [[COPY1]]
135-
; CHECK: STRHHui [[COPY2]], [[COPY]], 0 :: (store (s16))
134+
; CHECK: [[COPY2:%[0-9]+]]:fpr16 = COPY [[COPY1]].hsub
135+
; CHECK: STRHui [[COPY2]], [[COPY]], 0 :: (store (s16))
136136
%0:gpr(p0) = COPY $x0
137137
%1:fpr(s32) = COPY $s1
138138
%2:gpr(s32) = COPY %1

0 commit comments

Comments
 (0)