[rtl] Add flattening for xor, or, add, mul.

[cgyurgyik]

One thing to note is the amount of duplication in each case. To reduce this, I could do something like:

template <typename Inputs, typename Op>
auto flattenInputs(const Inputs &inputs, Op op,
                   size_t flattenedOpIndex) {
  auto flattenedOpInputs = op.inputs();
  SmallVector<Value, 4> newOperands;
  newOperands.reserve(inputs.size() + flattenedOpInputs.size());

  auto opPosition = inputs.begin() + flattenedOpIndex;
  newOperands.append(inputs.begin(), opPosition);
  newOperands.append(flattenedOpInputs.begin(), flattenedOpInputs.end());
  newOperands.append(opPosition + 1, inputs.end());
  return newOperands;
}

...

      // and(x, and(...)) -> and(x, ...) -- flatten
      for (size_t i = 0; i != size; ++i) {
        if (!inputs[i].hasOneUse())
          continue;
        auto andOp = inputs[i].getDefiningOp<rtl::AndOp>();
        if (!andOp)
          continue;
        rewriter.replaceOpWithNewOp<AndOp>(op, op.getType(),
                                           flattenInputs(inputs, andOp, i));
        return success();

I'm not sure if that's worth the extra level of abstraction or not, so looking for input.

[lattner]

I'd recommend pulling this out into a template like you propose. This gives you the excuse to give the logic a function name, and makes it more appetizing to reuse as new rtl ops are added.

Thanks for driving this!

[cgyurgyik]

Ok I've written something along those lines. We could make it slightly simpler by calling inputs[i].getDefiningOp<Op>() within the function as well, but that would mean calling it twice so I wasn't sure if that approach was any better. I'll wait to push until you've given your input (understanding that this week is the LLVM developers' meeting).

[lattner]

I thought you'd pull the whole "for loop" out, and template the function on the Operation type. I don't think this needs to be templated on Inputs, since that type should be the same for all of them.

Please mark this function static.

nit: if you keep it this way, please rename 'N' to something like 'splitNo' or something like that. If you keep 'n', please lower case it.

[cgyurgyik]

I thought about pulling out the entire loop, but then it would require (a) pulling in the rewriter and (b) determining whether it was a success() or a failure(). We could simply set the last pass to be flattening, i.e.

 LogicalResult matchAndRewrite(AndOp op,
                                  PatternRewriter &rewriter) const override {
   ...
   return flattenNthInput(inputs, rewriter); // returns success() or failure() depending on if there is an op to flatten.
}

However, I didn't want to solidify it as the last pass. I didn't know if there'd be any future issues of ordering, where maybe one pass should come before another for some reason or other.

[cgyurgyik]

Fixed static and changed name of N. I also wasn't sure whether a mlir::OperandRange was a view of a container or the container itself. Looking at other uses, it seems it's passed by value, so I mimicked this.

For this and everything above, if you disagree I will be happy to change it.

[lattner]

You don't think something like:

if (tryFlatteningOperands(op, rewriter)) return success();

would work? I would think that tryFlatteningOperands would be templated on the type of op, e.g. AndOp in this case.

[cgyurgyik]

That should work, let me brew something up and I'll request your review.