[llvm-commits] Review Requested

[Eli Friedman]

On Wed, May 30, 2012 at 7:08 PM, Joel Jones <joel_k_jones at apple.com> wrote:
> Allow ARM Codgen to generate a bic instruction, instead of a mov,and.
>
> Joel Jones
>
> Index: test/CodeGen/ARM/bicNoZext.ll
> ===================================================================
> --- test/CodeGen/ARM/bicNoZext.ll       (revision 0)
> +++ test/CodeGen/ARM/bicNoZext.ll       (revision 0)
> @@ -0,0 +1,19 @@
> +; RUN: llc %s -o - | FileCheck %s
> +; ModuleID = 'bic.c'
> +target triple = "thumbv7-apple-ios3.0.0"
> +
> +define zeroext i16 @foo16(i16 zeroext %f) nounwind readnone optsize ssp {
> +entry:
> +  ; CHECK: .thumb_func _foo16
> +  ; CHECK: {{bic[^#]*#3}}
> +  %and = and i16 %f, -4
> +  ret i16 %and
> +}
> +
> +define i32 @foo32(i32 %f) nounwind readnone optsize ssp {
> +entry:
> +  ; CHECK: .thumb_func _foo32
> +  ; CHECK: {{bic[^#]*#3}}
> +  %and = and i32 %f, -4
> +  ret i32 %and
> +}
> Index: include/llvm/Target/TargetSelectionDAG.td
> ===================================================================
> --- include/llvm/Target/TargetSelectionDAG.td   (revision 157726)
> +++ include/llvm/Target/TargetSelectionDAG.td   (working copy)
> @@ -362,6 +362,7 @@
>  def extractelt : SDNode<"ISD::EXTRACT_VECTOR_ELT", SDTVecExtract>;
>  def insertelt  : SDNode<"ISD::INSERT_VECTOR_ELT", SDTVecInsert>;
>
> +def assertzext : SDNode<"ISD::AssertZext", SDTIntUnaryOp>;
>
>  def fadd       : SDNode<"ISD::FADD"       , SDTFPBinOp, [SDNPCommutative]>;
>  def fsub       : SDNode<"ISD::FSUB"       , SDTFPBinOp>;
> Index: lib/Target/ARM/ARMInstrInfo.td
> ===================================================================
> --- lib/Target/ARM/ARMInstrInfo.td      (revision 157726)
> +++ lib/Target/ARM/ARMInstrInfo.td      (working copy)
> @@ -254,6 +254,14 @@
>   return CurDAG->getTargetConstant(~(int)N->getZExtValue(), MVT::i32);
>  }]>;
>
> +// so_imm_notSext_XFORM - Return a so_imm value packed into the format
> +// described for so_imm_notSext def below.
> +def so_imm_notSext_XFORM : SDNodeXForm<imm, [{
> +  APInt apIntN = N->getAPIntValue();
> +  unsigned N16bitSignExt = apIntN.trunc(16).sext(32).getZExtValue();
> +  return CurDAG->getTargetConstant(~N16bitSignExt, MVT::i32);
> +}]>;
> +
>  /// imm16_31 predicate - True if the 32-bit immediate is in the range [16,31].
>  def imm16_31 : ImmLeaf<i32, [{
>   return (int32_t)Imm >= 16 && (int32_t)Imm < 32;
> @@ -277,6 +285,15 @@
>   let ParserMatchClass = so_imm_not_asmoperand;
>  }
>
> +def so_imm_notSext : Operand<i32>, PatLeaf<(imm), [{
> +    APInt apIntN = N->getAPIntValue();
> +    if (!apIntN.isIntN(16)) return false;
> +    unsigned N16bitSignExt = apIntN.trunc(16).sext(32).getZExtValue();
> +    return ARM_AM::getSOImmVal(~N16bitSignExt) != -1;
> +  }], so_imm_notSext_XFORM> {
> +  let ParserMatchClass = so_imm_not_asmoperand;
> +}
> +
>  // sext_16_node predicate - True if the SDNode is sign-extended 16 or more bits.
>  def sext_16_node : PatLeaf<(i32 GPR:$a), [{
>   return CurDAG->ComputeNumSignBits(SDValue(N,0)) >= 17;
> @@ -3377,6 +3394,9 @@
>  def : ARMPat<(and   GPR:$src, so_imm_not:$imm),
>              (BICri GPR:$src, so_imm_not:$imm)>;
>
> +def : Pat<(and (i32 (assertzext GPR:$src)), so_imm_notSext:$imm),
> +             (BICri GPR:$src, so_imm_notSext:$imm)>;

You need to use ARMPat here; both BICri and so_imm_not make no sense
in Thumb mode.

You should use something like ComputeMaskedBits instead of explicitly
checking for assertzext.

When you implement this for Thumb mode, you should check it won't make
us choose longer Thumb2 encodings.

This is a very special case of the general transformation "X & C --> X
& (C|U) where X&U is zero and U is a constant which allows a shorter
encoding".  The general case is probably too much work to be worth
implementing... but there might be other cases to consider.

-Eli