[PATCH] D70841: [ARM][MVE] Sink vector shift operand

[Sam Parker via Phabricator via llvm-commits]

samparker marked an inline comment as done.
samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:4072
+
+def splat32_negated : PatFrag<(ops node:$val),
+                              (sub (ARMvmovImm 0), (ARMvdup node:$val))>;
----------------
dmgreen wrote:
> I think all of this should be happening in a DAG combine, to give the negate a chance to fold into something else.
> 
> It's generally not a good idea to create multiple outputs in a tablegen pattern. Ideally thy should be one/multi inputs to one output, with the other optimisations happening in dag combine to allow extra folding to happen.
> 
> I guess ideally this would happen much earlier, like in instcombine to allow all that folding goodness. Not sure how to make that happen sensibly though.
I thought that these negations were generated by us in lowering/combining? I haven't looked much into the lowering code, but I really would hope that we could remove some of the custom nodes... So I would be very hesitant to introduce yet more custom nodes, to get around the custom nodes that they've already introduced! I also don't see why more c++ would be better than having all the related patterns here, in a concise readable format.


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

https://reviews.llvm.org/D70841