[PATCH] D27779: [ARM] More aggressive matching for vpadd and vpaddl.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:9663
   // Attempt to create vpaddl for this add.
-  if (SDValue Result = AddCombineToVPADDL(N, N0, N1, DCI, Subtarget))
+  if (SDValue Result = AddCombineToVPADDLEarly(N, N0, N1, DCI, Subtarget))
+    return Result;
----------------
rengolin wrote:
> This looks weird. As much as I have long methods and nested `if`s, the interaction between `Early` and `Late` increases the coupling between them and make calling code more complex. (for example, you have an early exit on `Early` for later match on `Later`).
> 
> Adding an `if` block in `AddCombineToVPADDL` with an return would be much cleaner, if the condition is clear and concise, which should be the case, with your new helper.
They aren't really coupled like that; when I say "we'll match it to vpadd later", I mean, way later, like after the BUILD_VECTOR is lowered by legalization.  Maybe I could rename "Early" and "Late" to be a bit more clear...

Also, my eventual intent is to kill off AddCombineToVPADDLEarly; I'm just leaving it around to avoid regressing the fromExtendingExtractVectorElt_i8 and fromExtendingExtractVectorElt_i16 cases, which get lowered in stupid ways otherwise.


Repository:
  rL LLVM

https://reviews.llvm.org/D27779