[PATCH] D12887: [Machine Combiner] Refactor machine reassociation into a target-independent pass

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 17 11:11:19 PDT 2015


spatel added a comment.

In http://reviews.llvm.org/D12887#247990, @haicheng wrote:

> Thank you, Sanjay and Gerolf.
>
> This time I refactor into the machine combiner pass.  Verified again with llvm-test-suite and spec2006 that there is no functional change.


Thanks, Haicheng! This is much closer to what I imagined the refactoring would look like. :)

1. I don't think we need to distinguish between `useMachineCombiner` and `useMachineReassociation`. The reassociation patterns are just a subset of the combiner patterns. You can move what you are currently calling `getMachineReassociationPatterns` and `genAlterReassocCodeSequence` into the TargetInstrInfo base class implementations of `getMachineCombinerPatterns` and `genAlternativeCodeSequence`. Then the target overrides for those functions can choose to call the base implementation or not. When it's time to turn on the reassociation optimization for AArch, you just add a call to the base implementation ahead of the target-specific code that's already there. This will allow you to remove the `isReassociation` variable and associated if-checks in `combineInstructions`.

2. `reassociateOps` is still a bunch of target-independent duplicated code for PPC and x86. Can you pull that into MachineCombiner and make `setSpecialOperandAttr` a virtual function? It would only be overridden by x86 for now, but AArch might want to override that too.


Repository:
  rL LLVM

http://reviews.llvm.org/D12887





More information about the llvm-commits mailing list