[llvm] b82d18e - [AArch64][GlobalISel] Change G_FCONSTANTs feeding into stores into G_CONSTANTS

Jessica Paquette via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 16 15:19:02 PST 2020


Author: Jessica Paquette
Date: 2020-01-16T15:18:44-08:00
New Revision: b82d18e1e8e6a997f304cbf591e92af02e067fdb

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

LOG: [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

Added: 
    llvm/test/CodeGen/AArch64/GlobalISel/combine-fconstant.mir

Modified: 
    llvm/lib/Target/AArch64/AArch64Combine.td
    llvm/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/AArch64/AArch64Combine.td b/llvm/lib/Target/AArch64/AArch64Combine.td
index bb99f2516ecf..e42556b5901a 100644
--- a/llvm/lib/Target/AArch64/AArch64Combine.td
+++ b/llvm/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/llvm/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp b/llvm/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp
index 230fd514d022..e85f8757866b 100644
--- a/llvm/lib/Target/AArch64/AArch64PreLegalizerCombiner.cpp
+++ b/llvm/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/llvm/test/CodeGen/AArch64/GlobalISel/combine-fconstant.mir b/llvm/test/CodeGen/AArch64/GlobalISel/combine-fconstant.mir
new file mode 100644
index 000000000000..49e6b0d7264f
--- /dev/null
+++ b/llvm/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
+...


        


More information about the llvm-commits mailing list