[PATCH] D143283: [Transform][InstCombine]: transform lshr pattern.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen requested changes to this revision.
sdesmalen added a comment.
This revision now requires changes to proceed.

This patch doesn't seem like it is in a state to review yet (e.g. it doesn't have any tests), perhaps you can add 'WIP' to the title to make it clear that this is a Work In Progress?



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp:1420-1434
+  // (A >> 1) + (B >> 1) + (A&B)&1
+  if (match(Op1, m_APInt(C))) {
+    unsigned ShAmtC = C->getZExtValue();
+    if(1 == ShAmtC) {
+      const APInt *constantInt = nullptr;
+      // Op0: add (X, 1)
+      if(match(Op0,
----------------
Just as a suggestion for future uses of these pattern matches, is that you could write this in a more compact way like this:

  if (match(Op1, m_SpecificInt(1)) &&
      match(Op2, m_Add(m_Add(m_OneUse(m_Value(A)), m_OneUse(m_Value(B))),
                       m_ConstantInt(C))) &&
      (C->getZExtValue() == 0 || C->getZExtValue() == 1)) {
    ...
  }



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp:1456
+            Value *Add1 = Builder.CreateAdd(ALshr, BLshr);
+            auto *newInstr = BinaryOperator::CreateAdd(Add1, AB1);
+            return newInstr;
----------------
This transform is only profitable when the target has no explicit instructions for this pattern (e.g. like Arm's urhadd/srhadd) and the extends are expensive. The InstCombine pass transforms the IR to a canonical form and doesn't use the CostModel to decide whether a certain form is profitable or not. A better place to do this would be in an DAGCombine (to start it's good enough for this to be target-specific, e.g. a combine in AArch64ISelLowering.cpp) where we can simply check if the target being compiled for has the urhadd/shradd instructions, and if not, do the transform.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143283/new/

https://reviews.llvm.org/D143283