[llvm] 2ed8053 - Revert "[AArch64][GlobalISel] Don't contract cross-bank copies into truncating stores."

[Amara Emerson via llvm-commits]

Author: Amara Emerson
Date: 2021-08-24T16:26:56-07:00
New Revision: 2ed8053d465e169e9819c933dea1a319a8d19533

URL: https://github.com/llvm/llvm-project/commit/2ed8053d465e169e9819c933dea1a319a8d19533
DIFF: https://github.com/llvm/llvm-project/commit/2ed8053d465e169e9819c933dea1a319a8d19533.diff

LOG: Revert "[AArch64][GlobalISel] Don't contract cross-bank copies into truncating stores."

This reverts commit 67bf3ac7446bd8ba16bb5ef82fa9f28a848616da.

The reason is that this change is now superseded by 04fb9b729a53 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.

Added: 
    

Modified: 
    llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
    llvm/test/CodeGen/AArch64/GlobalISel/contract-store.mir

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
index 6b22c3329c7e..c2b8b58fbec7 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
@@ -105,7 +105,7 @@ class AArch64InstructionSelector : public InstructionSelector {
   bool earlySelectSHL(MachineInstr &I, MachineRegisterInfo &MRI);
 
   /// Eliminate same-sized cross-bank copies into stores before selectImpl().
-  bool contractCrossBankCopyIntoStore(GStore &I,
+  bool contractCrossBankCopyIntoStore(MachineInstr &I,
                                       MachineRegisterInfo &MRI);
 
   bool convertPtrAddToAdd(MachineInstr &I, MachineRegisterInfo &MRI);
@@ -1943,9 +1943,8 @@ bool AArch64InstructionSelector::preISelLower(MachineInstr &I) {
     return true;
   }
   case TargetOpcode::G_STORE: {
-    auto &StoreMI = cast<GStore>(I);
-    bool Changed = contractCrossBankCopyIntoStore(StoreMI, MRI);
-    MachineOperand &SrcOp = StoreMI.getOperand(0);
+    bool Changed = contractCrossBankCopyIntoStore(I, MRI);
+    MachineOperand &SrcOp = I.getOperand(0);
     if (MRI.getType(SrcOp.getReg()).isPointer()) {
       // Allow matching with imported patterns for stores of pointers. Unlike
       // 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) {
       RBI.constrainGenericRegister(NewSrc, AArch64::GPR64RegClass, MRI);
       Changed = true;
     }
-#if 0
-    // Now look for truncating stores to the FPR bank. We don't support these,
-    // but since truncating store formation happens before RBS, we can only
-    // split them up again here. We don't want to assign truncstores to GPR only
-    // since that would have a perf impact due to extra moves.
-    LLT SrcTy = MRI.getType(SrcReg);
-    if (RBI.getRegBank(SrcReg, MRI, TRI)->getID() == AArch64::FPRRegBankID) {
-      if (SrcTy.isScalar() &&
-          SrcTy.getSizeInBits() > StoreMI.getMemSizeInBits()) {
-        // Generate an explicit truncate and make this into a non-truncating
-        // store.
-        auto Trunc =
-            MIB.buildTrunc(LLT::scalar(StoreMI.getMemSizeInBits()), SrcReg);
-        MRI.setRegBank(Trunc.getReg(0), RBI.getRegBank(AArch64::FPRRegBankID));
-        if (!select(*Trunc)) {
-          return false;
-        }
-        SrcOp.setReg(Trunc.getReg(0));
-        return true;
-      }
-    }
-#endif
     return Changed;
   }
   case TargetOpcode::G_PTR_ADD:
@@ -2113,7 +2090,8 @@ bool AArch64InstructionSelector::earlySelectSHL(MachineInstr &I,
 }
 
 bool AArch64InstructionSelector::contractCrossBankCopyIntoStore(
-    GStore &StoreMI, MachineRegisterInfo &MRI) {
+    MachineInstr &I, MachineRegisterInfo &MRI) {
+  assert(I.getOpcode() == TargetOpcode::G_STORE && "Expected G_STORE");
   // If we're storing a scalar, it doesn't matter what register bank that
   // scalar is on. All that matters is the size.
   //
@@ -2128,11 +2106,11 @@ bool AArch64InstructionSelector::contractCrossBankCopyIntoStore(
   // G_STORE %x:gpr(s32)
   //
   // And then continue the selection process normally.
-  Register DefDstReg = getSrcRegIgnoringCopies(StoreMI.getValueReg(), MRI);
+  Register DefDstReg = getSrcRegIgnoringCopies(I.getOperand(0).getReg(), MRI);
   if (!DefDstReg.isValid())
     return false;
   LLT DefDstTy = MRI.getType(DefDstReg);
-  Register StoreSrcReg = StoreMI.getValueReg();
+  Register StoreSrcReg = I.getOperand(0).getReg();
   LLT StoreSrcTy = MRI.getType(StoreSrcReg);
 
   // If we get something strange like a physical register, then we shouldn't
@@ -2144,16 +2122,12 @@ bool AArch64InstructionSelector::contractCrossBankCopyIntoStore(
   if (DefDstTy.getSizeInBits() != StoreSrcTy.getSizeInBits())
     return false;
 
-  // Is this store a truncating one?
-  if (StoreSrcTy.getSizeInBits() != StoreMI.getMemSizeInBits())
-    return false;
-
   if (RBI.getRegBank(StoreSrcReg, MRI, TRI) ==
       RBI.getRegBank(DefDstReg, MRI, TRI))
     return false;
 
   // We have a cross-bank copy, which is entering a store. Let's fold it.
-  StoreMI.getOperand(0).setReg(DefDstReg);
+  I.getOperand(0).setReg(DefDstReg);
   return true;
 }
 

diff  --git a/llvm/test/CodeGen/AArch64/GlobalISel/contract-store.mir b/llvm/test/CodeGen/AArch64/GlobalISel/contract-store.mir
index cb92b6a58ba5..1955aa935cd9 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/contract-store.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/contract-store.mir
@@ -131,8 +131,8 @@ body:             |
     ; CHECK-LABEL: name: contract_s16_truncstore
     ; CHECK: [[COPY:%[0-9]+]]:gpr64sp = COPY $x0
     ; CHECK: [[COPY1:%[0-9]+]]:fpr32 = COPY $s1
-    ; CHECK: [[COPY2:%[0-9]+]]:gpr32 = COPY [[COPY1]]
-    ; CHECK: STRHHui [[COPY2]], [[COPY]], 0 :: (store (s16))
+    ; CHECK: [[COPY2:%[0-9]+]]:fpr16 = COPY [[COPY1]].hsub
+    ; CHECK: STRHui [[COPY2]], [[COPY]], 0 :: (store (s16))
     %0:gpr(p0) = COPY $x0
     %1:fpr(s32) = COPY $s1
     %2:gpr(s32) = COPY %1