From b82d18e1e8e6a997f304cbf591e92af02e067fdb Mon Sep 17 00:00:00 2001 From: Jessica Paquette 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 --- llvm/lib/Target/AArch64/AArch64Combine.td | 9 ++- .../Target/AArch64/AArch64PreLegalizerCombiner.cpp | 26 ++++++++ .../AArch64/GlobalISel/combine-fconstant.mir | 73 ++++++++++++++++++++++ 3 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 llvm/test/CodeGen/AArch64/GlobalISel/combine-fconstant.mir diff --git a/llvm/lib/Target/AArch64/AArch64Combine.td b/llvm/lib/Target/AArch64/AArch64Combine.td index bb99f25..e42556b 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 230fd51..e85f875 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 0000000..49e6b0d --- /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 +... -- 2.7.4