From 5ae7bf2acaca13b99d65d289667161308ec68e83 Mon Sep 17 00:00:00 2001
From: Jessica Paquette <jpaquette@apple.com>
Date: Wed, 15 Jan 2020 16:20:29 -0800
Subject: [PATCH] [AArch64][GlobalISel] Change G_FCONSTANTs feeding into stores
 into G_CONSTANTS

Given the following situation:

x = G_FCONSTANT (something that can't be materialized)
G_STORE x, some_addr

We know that x must be materialized as at least a single mov. However, at the
time of selection, the G_STORE will have been regbankselected to a FPR store.

So, as a result, you'll get an unnecessary fmov into the G_STORE.

Storing a constant value in a GPR and a constant value in a FPR are the same.
So, whenever you see a G_FCONSTANT that feeds into only G_STORES, so might as
well make it a G_CONSTANT.

This adds a target-specific combine which changes G_FCONSTANTs feeding into
G_STOREs into G_CONSTANTs.

Differential Revision: https://reviews.llvm.org/D72814
---
 lib/Target/AArch64/AArch64Combine.td          |  9 ++-
 .../AArch64/AArch64PreLegalizerCombiner.cpp   | 26 +++++++
 .../AArch64/GlobalISel/combine-fconstant.mir  | 73 +++++++++++++++++++
 3 files changed, 107 insertions(+), 1 deletion(-)
 create mode 100644 test/CodeGen/AArch64/GlobalISel/combine-fconstant.mir

diff --git a/lib/Target/AArch64/AArch64Combine.td b/lib/Target/AArch64/AArch64Combine.td
index bb99f2516ec..e42556b5901 100644
--- a/lib/Target/AArch64/AArch64Combine.td
+++ b/lib/Target/AArch64/AArch64Combine.td
@@ -11,8 +11,15 @@
 
 include "llvm/Target/GlobalISel/Combine.td"
 
+def fconstant_to_constant : GICombineRule<
+  (defs root:$root),
+  (match (wip_match_opcode G_FCONSTANT):$root,
+         [{ return matchFConstantToConstant(*${root}, MRI); }]),
+  (apply [{ applyFConstantToConstant(*${root}); }])>;
+
 def AArch64PreLegalizerCombinerHelper: GICombinerHelper<
   "AArch64GenPreLegalizerCombinerHelper", [all_combines,
-                                           elide_br_by_inverting_cond]> {
+                                           elide_br_by_inverting_cond,
+                                           fconstant_to_constant]> {
   let DisableRuleOption = "aarch64prelegalizercombiner-disable-rule";
 }
diff --git a/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp b/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp
index 230fd514d02..e85f8757866 100644
--- a/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp
+++ b/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp
@@ -27,6 +27,32 @@
 using namespace llvm;
 using namespace MIPatternMatch;
 
+/// Return true if a G_FCONSTANT instruction is known to be better-represented
+/// as a G_CONSTANT.
+static bool matchFConstantToConstant(MachineInstr &MI,
+                                     MachineRegisterInfo &MRI) {
+  assert(MI.getOpcode() == TargetOpcode::G_FCONSTANT);
+  Register DstReg = MI.getOperand(0).getReg();
+  const unsigned DstSize = MRI.getType(DstReg).getSizeInBits();
+  if (DstSize != 32 && DstSize != 64)
+    return false;
+
+  // When we're storing a value, it doesn't matter what register bank it's on.
+  // Since not all floating point constants can be materialized using a fmov,
+  // it makes more sense to just use a GPR.
+  return all_of(MRI.use_instructions(DstReg),
+                [](const MachineInstr &Use) { return Use.mayStore(); });
+}
+
+/// Change a G_FCONSTANT into a G_CONSTANT.
+static void applyFConstantToConstant(MachineInstr &MI) {
+  assert(MI.getOpcode() == TargetOpcode::G_FCONSTANT);
+  MachineIRBuilder MIB(MI);
+  const APFloat &ImmValAPF = MI.getOperand(1).getFPImm()->getValueAPF();
+  MIB.buildConstant(MI.getOperand(0).getReg(), ImmValAPF.bitcastToAPInt());
+  MI.eraseFromParent();
+}
+
 #define AARCH64PRELEGALIZERCOMBINERHELPER_GENCOMBINERHELPER_DEPS
 #include "AArch64GenGICombiner.inc"
 #undef AARCH64PRELEGALIZERCOMBINERHELPER_GENCOMBINERHELPER_DEPS
diff --git a/test/CodeGen/AArch64/GlobalISel/combine-fconstant.mir b/test/CodeGen/AArch64/GlobalISel/combine-fconstant.mir
new file mode 100644
index 00000000000..49e6b0d7264
--- /dev/null
+++ b/test/CodeGen/AArch64/GlobalISel/combine-fconstant.mir
@@ -0,0 +1,73 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -run-pass=aarch64-prelegalizer-combiner -verify-machineinstrs -mtriple aarch64-unknown-unknown %s -o - | FileCheck %s
+...
+---
+name:            fconstant_to_constant_s32
+alignment:       4
+tracksRegLiveness: true
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $x0
+    ; Only feeding into stores here. Also, the value can't be materialized using
+    ; fmov, so it's strictly better to use a mov.
+    ; CHECK-LABEL: name: fconstant_to_constant_s32
+    ; CHECK: liveins: $x0
+    ; CHECK: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
+    ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 1028443341
+    ; CHECK: [[C1:%[0-9]+]]:_(s64) = G_CONSTANT i64 524
+    ; CHECK: [[PTR_ADD:%[0-9]+]]:_(p0) = G_PTR_ADD [[COPY]], [[C1]](s64)
+    ; CHECK: G_STORE [[C]](s32), [[PTR_ADD]](p0) :: (store 4)
+    ; CHECK: RET_ReallyLR
+    %0:_(p0) = COPY $x0
+    %3:_(s32) = G_FCONSTANT float 0x3FA99999A0000000
+    %1:_(s64) = G_CONSTANT i64 524
+    %2:_(p0) = G_PTR_ADD %0, %1(s64)
+    G_STORE %3(s32), %2(p0) :: (store 4)
+    RET_ReallyLR
+...
+---
+name:            fconstant_to_constant_s64
+alignment:       4
+tracksRegLiveness: true
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $x0
+    ; CHECK-LABEL: name: fconstant_to_constant_s64
+    ; CHECK: liveins: $x0
+    ; CHECK: %ptr:_(p0) = COPY $x0
+    ; CHECK: %c:_(s64) = G_CONSTANT i64 0
+    ; CHECK: G_STORE %c(s64), %ptr(p0) :: (store 8)
+    ; CHECK: RET_ReallyLR
+    %ptr:_(p0) = COPY $x0
+    %c:_(s64) = G_FCONSTANT double 0.0
+    G_STORE %c(s64), %ptr(p0) :: (store 8)
+    RET_ReallyLR
+...
+---
+name:            no_store_means_no_combine
+alignment:       4
+tracksRegLiveness: true
+frameInfo:
+  maxAlignment:    1
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $x0, $x1
+    ; When we aren't feeding into a store, the combine shouldn't happen.
+    ; CHECK-LABEL: name: no_store_means_no_combine
+    ; CHECK: liveins: $x0, $x1
+    ; CHECK: %v:_(s64) = COPY $x0
+    ; CHECK: %c:_(s64) = G_FCONSTANT double 0.000000e+00
+    ; CHECK: %add:_(s64) = G_FADD %v, %c
+    ; CHECK: RET_ReallyLR implicit %add(s64)
+    %v:_(s64) = COPY $x0
+    %c:_(s64) = G_FCONSTANT double 0.0
+    %add:_(s64) = G_FADD %v, %c
+    RET_ReallyLR implicit %add
+...