[AArch64][GlobalISel] Add some minor post-selection optimizations.

[aemerson]

Summary

The first is doing some trivial cross-regclass folding, where we can either
do some extra constraining to eliminate the copy or modify uses to use a smaller
regclass.

The second is that we set the kill flags on vreg use operands if they're the
only use of that vreg.

Both of these cases seem to be done by SelectionDAG's lowering from DAG to
MIs that we currently miss. There are minor code size improvements on average.

Here's the results for -Os on CTMark.

Program                                       size.__text
                                              before         after           diff
tramp3d-v4/tramp3d-v4                         366000.00      366012.00       0.0%
mafft/pairlocalalign                          248196.00      248188.00      -0.0%
7zip/7zip-benchmark                           568612.00      568592.00      -0.0%
kimwitu++/kc                                  434704.00      434676.00      -0.0%
Bullet/bullet                                 456128.00      456096.00      -0.0%
sqlite3/sqlite3                               284136.00      284100.00      -0.0%
ClamAV/clamscan                               381492.00      381396.00      -0.0%
SPASS/SPASS                                   412052.00      411944.00      -0.0%
lencod/lencod                                 428060.00      427912.00      -0.0%
consumer-typeset/consumer-typeset             413148.00      411116.00      -0.5%
                           Geomean difference                               -0.1%

[arsenm]

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

89

I'd be disappointed if this is actually helping. Do you have examples where kill flags are needed that aren't added by LiveVariables?

[paquette]

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

94

Doxygen comments should go in the header, right?

108

isPhysical?

132

We need the one-use check here right?

150

I think you can just use explicit_operands() here?

156

[aemerson]

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

89

Huh, you're right. I'm not sure why I thought this was making an improvement, must have been measuring the wrong change. I'll remove this from the patch.

108

Registers can be either physical, virtual or stack slots apparently, so it's better to use isVirtual().

132

No, if the COPY source register is a subclass of the destination, we can just replace all uses of the dest with the source safely, so we can do it for every use of the COPY's dst.

150

I need Idx for the getOperandConstraint() call below.

[aemerson]

Address comments and remove the kill flags code.

[arsenm]

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

108

I'm pretty sure the stack slot case only happens transiently in regalloc

[aemerson]

Use isPhysical()

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

 //=== AArch64PostSelectOptimize.cpp ---------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This pass does post-instruction-selection optimizations in the GlobalISel
 // pipeline, before the rest of codegen runs.
 //
 //===----------------------------------------------------------------------===//
 
 #include "AArch64.h"
 #include "AArch64TargetMachine.h"
 #include "MCTargetDesc/AArch64MCTargetDesc.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/CodeGen/GlobalISel/Utils.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/MachineInstr.h"
 #include "llvm/CodeGen/MachineOperand.h"
 #include "llvm/CodeGen/TargetPassConfig.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/ErrorHandling.h"
 
 #define DEBUG_TYPE "aarch64-post-select-optimize"
 
 using namespace llvm;
 
 namespace {
 class AArch64PostSelectOptimize : public MachineFunctionPass {
 public:
   static char ID;
 
   AArch64PostSelectOptimize();
 
   StringRef getPassName() const override {
     return "AArch64 Post Select Optimizer";
   }
 
   bool runOnMachineFunction(MachineFunction &MF) override;
 
   void getAnalysisUsage(AnalysisUsage &AU) const override;
 
 private:
   bool optimizeNZCVDefs(MachineBasicBlock &MBB);
+  bool doPeepholeOpts(MachineBasicBlock &MBB);
+  /// Look for cross regclass copies that can be trivially eliminated.
+  bool foldSimpleCrossClassCopies(MachineInstr &MI);
 };
 } // end anonymous namespace
 
 void AArch64PostSelectOptimize::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<TargetPassConfig>();
   AU.setPreservesCFG();
   getSelectionDAGFallbackAnalysisUsage(AU);
   MachineFunctionPass::getAnalysisUsage(AU);
case AArch64::SUBSXrs:
     return AArch64::SUBXrs;
   case AArch64::SUBSXri:
     return AArch64::SUBXri;
   case AArch64::SUBSWri:
     return AArch64::SUBWri;
   }
 }
 
+bool AArch64PostSelectOptimize::doPeepholeOpts(MachineBasicBlock &MBB) {
+  bool Changed = false;
+  for (auto &MI : make_early_inc_range(make_range(MBB.begin(), MBB.end()))) {
+    Changed |= foldSimpleCrossClassCopies(MI);
+  }
+  return Changed;
+}
+
+bool AArch64PostSelectOptimize::foldSimpleCrossClassCopies(MachineInstr &MI) {
+  auto *MF = MI.getMF();
+  auto &MRI = MF->getRegInfo();
+
+  if (!MI.isCopy())
+    return false;
+
+  if (MI.getOperand(1).getSubReg())
+    return false; // Don't deal with subreg copies
+
+  Register Src = MI.getOperand(1).getReg();
+  Register Dst = MI.getOperand(0).getReg();
+
+  if (Src.isPhysical() || Dst.isPhysical())
+    return false;
+
+  const TargetRegisterClass *SrcRC = MRI.getRegClass(Src);
+  const TargetRegisterClass *DstRC = MRI.getRegClass(Dst);
+
+  if (SrcRC == DstRC)
+    return false;
+
+
+  if (SrcRC->hasSubClass(DstRC)) {
+    // This is the case where the source class is a superclass of the dest, so
+    // if the copy is the only user of the source, we can just constrain the
+    // source reg to the dest class.
+
+    if (!MRI.hasOneNonDBGUse(Src))
+      return false; // Only constrain single uses of the source.
+
+    // Constrain to dst reg class as long as it's not a weird class that only
+    // has a few registers.
+    if (!MRI.constrainRegClass(Src, DstRC, /* MinNumRegs */ 25))
+      return false;
+  } else if (DstRC->hasSubClass(SrcRC)) {
+    // This is the inverse case, where the destination class is a superclass of
+    // the source. Here, if the copy is the only user, we can just constrain
+    // the user of the copy to use the smaller class of the source.
+  } else {
+    return false;
+  }
+
+  MRI.replaceRegWith(Dst, Src);
+  MI.eraseFromParent();
+  return true;
+}
+
 bool AArch64PostSelectOptimize::optimizeNZCVDefs(MachineBasicBlock &MBB) {
   // Consider the following code:
   //  FCMPSrr %0, %1, implicit-def $nzcv
   //  %sel1:gpr32 = CSELWr %_, %_, 12, implicit $nzcv
   //  %sub:gpr32 = SUBSWrr %_, %_, implicit-def $nzcv
   //  FCMPSrr %0, %1, implicit-def $nzcv
   //  %sel2:gpr32 = CSELWr %_, %_, 12, implicit $nzcv
   // This kind of code where we have 2 FCMPs each feeding a CSEL can happen
   // when we have a single IR fcmp being used by two selects. During selection,
   // to ensure that there can be no clobbering of nzcv between the fcmp and the
   // csel, we have to generate an fcmp immediately before each csel is
   // selected.
   // However, often we can essentially CSE these together later in MachineCSE.
   // This doesn't work though if there are unrelated flag-setting instructions
   // in between the two FCMPs. In this case, the SUBS defines NZCV
   // but it doesn't have any users, being overwritten by the second FCMP.
   //
   // Our solution here is to try to convert flag setting operations between
   // a interval of identical FCMPs, so that CSE will be able to eliminate one.
   bool Changed = false;
   auto &MF = *MBB.getParent();
   auto &Subtarget = MF.getSubtarget();
   const auto &TII = Subtarget.getInstrInfo();
   auto TRI = Subtarget.getRegisterInfo();
   auto RBI = Subtarget.getRegBankInfo();
   auto &MRI = MF.getRegInfo();
bool AArch64PostSelectOptimize::runOnMachineFunction(MachineFunction &MF) {
   if (MF.getProperties().hasProperty(
           MachineFunctionProperties::Property::FailedISel))
     return false;
   assert(MF.getProperties().hasProperty(
              MachineFunctionProperties::Property::Selected) &&
          "Expected a selected MF");
 
   bool Changed = false;
-  for (auto &BB : MF)
+  for (auto &BB : MF) {
     Changed |= optimizeNZCVDefs(BB);
+    Changed |= doPeepholeOpts(BB);
+  }
   return Changed;
 }
 
 char AArch64PostSelectOptimize::ID = 0;
 INITIALIZE_PASS_BEGIN(AArch64PostSelectOptimize, DEBUG_TYPE,
                       "Optimize AArch64 selected instructions",
                       false, false)
 INITIALIZE_PASS_END(AArch64PostSelectOptimize, DEBUG_TYPE,
                     "Optimize AArch64 selected instructions", false,
                     false)
 
 namespace llvm {
 FunctionPass *createAArch64PostSelectOptimize() {
   return new AArch64PostSelectOptimize();
 }
 } // end namespace llvm

llvm/test/CodeGen/AArch64/GlobalISel/postselectopt-xclass-copies.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple aarch64 -run-pass=aarch64-post-select-optimize -verify-machineinstrs %s -o - | FileCheck %s
+--- |
+  target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+
+  @x = external hidden local_unnamed_addr global i32*, align 8
+  define void @copy_from_larger_rc_def() { ret void }
+  define void @copy_from_larger_rc_def_multi_use() { ret void }
+  define void @copy_from_smaller_rc_def() { ret void }
+
+...
+---
+name:            copy_from_larger_rc_def
+alignment:       4
+legalized:       true
+regBankSelected: true
+selected:        true
+tracksRegLiveness: true
+liveins:
+  - { reg: '$x0' }
+  - { reg: '$w1' }
+  - { reg: '$x2' }
+body:             |
+  bb.1:
+    liveins: $w1, $x0, $x2
+
+    ; Show that if we're doing a copy from a large rc to a single user with a smaller rc
+    ; then we just constrain the def instead.
+    ; CHECK-LABEL: name: copy_from_larger_rc_def
+    ; CHECK: liveins: $w1, $x0, $x2
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr64 = COPY $x0
+    ; CHECK-NEXT: %large_rc_def:gpr64common = UBFMXri [[COPY]], 61, 60
+    ; CHECK-NEXT: %add:gpr64sp = ADDXri %large_rc_def, 3, 0
+    ; CHECK-NEXT: $x0 = COPY %add
+    ; CHECK-NEXT: RET_ReallyLR
+    %0:gpr64 = COPY $x0
+    %large_rc_def:gpr64 = UBFMXri %0, 61, 60
+    %constrain_copy:gpr64common = COPY %large_rc_def
+    ; Even though ADDXri may not actually need to use gpr64common, just use it as an example.
+    %add:gpr64sp = ADDXri %constrain_copy, 3, 0
+    $x0 = COPY %add
+    RET_ReallyLR
+
+...
+---
+name:            copy_from_larger_rc_def_multi_use
+alignment:       4
+legalized:       true
+regBankSelected: true
+selected:        true
+tracksRegLiveness: true
+liveins:
+  - { reg: '$x0' }
+  - { reg: '$w1' }
+  - { reg: '$x2' }
+body:             |
+  bb.1:
+    liveins: $w1, $x0, $x2
+
+    ; Don't constrain def if the original def has multiple users.
+    ; CHECK-LABEL: name: copy_from_larger_rc_def_multi_use
+    ; CHECK: liveins: $w1, $x0, $x2
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr64 = COPY $x0
+    ; CHECK-NEXT: %large_rc_def:gpr64 = UBFMXri [[COPY]], 61, 60
+    ; CHECK-NEXT: %constrain_copy:gpr64common = COPY %large_rc_def
+    ; CHECK-NEXT: %add:gpr64sp = ADDXri %constrain_copy, 3, 0
+    ; CHECK-NEXT: %add2:gpr64sp = ADDXri %constrain_copy, 3, 0
+    ; CHECK-NEXT: $x0 = COPY %add
+    ; CHECK-NEXT: $x1 = COPY %large_rc_def
+    ; CHECK-NEXT: RET_ReallyLR
+    %0:gpr64 = COPY $x0
+    %large_rc_def:gpr64 = UBFMXri %0, 61, 60
+    %constrain_copy:gpr64common = COPY %large_rc_def
+    %add:gpr64sp = ADDXri %constrain_copy, 3, 0
+    %add2:gpr64sp = ADDXri %constrain_copy, 3, 0
+    $x0 = COPY %add
+    $x1 = COPY %large_rc_def
+    RET_ReallyLR
+
+...
+---
+name:            copy_from_smaller_rc_def
+alignment:       4
+legalized:       true
+regBankSelected: true
+selected:        true
+tracksRegLiveness: true
+liveins:
+  - { reg: '$x0' }
+  - { reg: '$w1' }
+  - { reg: '$x2' }
+body:             |
+  bb.1:
+    liveins: $w1, $x0, $x2
+
+    ; Show that if we're doing a copy from a small rc to a single user with a larger rc
+    ; then we just use the smaller def instead of doing a copy.
+    ; CHECK-LABEL: name: copy_from_smaller_rc_def
+    ; CHECK: liveins: $w1, $x0, $x2
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr64common = COPY $x0
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr64common = COPY $x1
+    ; CHECK-NEXT: %add:gpr64common = ADDXri [[COPY1]], 3, 0
+    ; CHECK-NEXT: STRXui [[COPY1]], %add, target-flags(aarch64-pageoff, aarch64-nc) @x :: (store (p0))
+    ; CHECK-NEXT: RET_ReallyLR
+    %0:gpr64common = COPY $x0
+    %1:gpr64common = COPY $x1
+
+    %add:gpr64common = ADDXri %1, 3, 0
+    %copy:gpr64sp = COPY %add
+    STRXui %1, %copy, target-flags(aarch64-pageoff, aarch64-nc) @x :: (store (p0))
+    RET_ReallyLR
+
+...