Page MenuHomePhabricator

[LoongArch] Offset folding for frameindex
ClosedPublic

Authored by wangleiat on Jul 21 2022, 4:25 AM.

Details

Diff Detail

Event Timeline

wangleiat created this revision.Jul 21 2022, 4:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 21 2022, 4:25 AM
wangleiat requested review of this revision.Jul 21 2022, 4:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 21 2022, 4:25 AM

Do you know why csky/hexagon need isOrEquivalentToAdd while other targets can produce efficient code without isOrEquivalentToAdd ?

barannikov88 added a subscriber: barannikov88.EditedJul 23 2022, 2:39 PM

Do you know why csky/hexagon need isOrEquivalentToAdd while other targets can produce efficient code without isOrEquivalentToAdd ?

Other targets just do that manually in *ISelDAGToDAG.cpp file. E.g. X86:

case ISD::OR:
  // We want to look through a transform in InstCombine and DAGCombiner that
  // turns 'add' into 'or', so we can treat this 'or' exactly like an 'add'.
  // Example: (or (and x, 1), (shl y, 3)) --> (add (and x, 1), (shl y, 3))
  // An 'lea' can then be used to match the shift (multiply) and add:
  // and $1, %esi
  // lea (%rsi, %rdi, 8), %rax
  if (CurDAG->haveNoCommonBitsSet(N.getOperand(0), N.getOperand(1)) &&
      !matchAdd(N, AM, Depth))
    return false;
  break;

ARM:

// Determine whether an ISD::OR's operands are suitable to turn the operation
// into an addition, which often has more compact encodings.
bool ARMDAGToDAGISel::SelectAddLikeOr(SDNode *Parent, SDValue N, SDValue &Out) {
  assert(Parent->getOpcode() == ISD::OR && "unexpected parent");
  Out = N;
  return CurDAG->haveNoCommonBitsSet(N, Parent->getOperand(1));
}
barannikov88 added inline comments.Jul 23 2022, 2:43 PM
llvm/lib/Target/LoongArch/LoongArchInstrInfo.td
591

You can check for both ADD and OR in one PatFrags, e.g.:

def add_like : PatFrags<(ops node:$lhs, node:$rhs),
                        [(add $lhs, $rhs), (or $lhs, $rhs)], [{
  return N->getOpcode() == ISD::ADD || isOrEquivalentToAdd(N);
}]>;

This will halve the number of patterns. Up to you, of course.

Do you know why csky/hexagon need isOrEquivalentToAdd while other targets can produce efficient code without isOrEquivalentToAdd ?

Other targets just do that manually in *ISelDAGToDAG.cpp file. E.g. X86:

case ISD::OR:
  // We want to look through a transform in InstCombine and DAGCombiner that
  // turns 'add' into 'or', so we can treat this 'or' exactly like an 'add'.
  // Example: (or (and x, 1), (shl y, 3)) --> (add (and x, 1), (shl y, 3))
  // An 'lea' can then be used to match the shift (multiply) and add:
  // and $1, %esi
  // lea (%rsi, %rdi, 8), %rax
  if (CurDAG->haveNoCommonBitsSet(N.getOperand(0), N.getOperand(1)) &&
      !matchAdd(N, AM, Depth))
    return false;
  break;

ARM:

// Determine whether an ISD::OR's operands are suitable to turn the operation
// into an addition, which often has more compact encodings.
bool ARMDAGToDAGISel::SelectAddLikeOr(SDNode *Parent, SDValue N, SDValue &Out) {
  assert(Parent->getOpcode() == ISD::OR && "unexpected parent");
  Out = N;
  return CurDAG->haveNoCommonBitsSet(N, Parent->getOperand(1));
}

Agree, The DAGCombiner will almost certainly be generated ISD::OR operator for frameindex. (natural alignment attribute?)

llvm/lib/Target/LoongArch/LoongArchInstrInfo.td
591

You can check for both ADD and OR in one PatFrags, e.g.:

def add_like : PatFrags<(ops node:$lhs, node:$rhs),
                        [(add $lhs, $rhs), (or $lhs, $rhs)], [{
  return N->getOpcode() == ISD::ADD || isOrEquivalentToAdd(N);
}]>;

This will halve the number of patterns. Up to you, of course.

Thanks, I will do that.

wangleiat updated this revision to Diff 447293.Jul 25 2022, 6:00 AM

Address @barannikov88's comment.

barannikov88 accepted this revision.Jul 25 2022, 6:48 AM

Looks good to me, although I'm not familiar with the architecture.

This revision is now accepted and ready to land.Jul 25 2022, 6:48 AM
This revision was landed with ongoing or failed builds.Jul 29 2022, 2:28 AM
This revision was automatically updated to reflect the committed changes.