[PATCH] D114174: [ARM][CodeGen] Add support for complex deinterleaving

[Nicholas Guy via Phabricator via llvm-commits]

NickGuy marked 15 inline comments as done.
NickGuy added inline comments.


================
Comment at: llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp:593
+
+  for (auto &I : *ConvergingI->getParent()) {
+
----------------
chill wrote:
> Why are we scanning the whole block?
> It looks to me (admittedly I don't understand what this pass does yet)
> we can just walk over the `Instructions` vector (in reverse, if the order matters),
> avoiding the quadratic complexity.
The order of the `CompositeNode`s is important, this was a holdover from before we were sorting the nodes. 


================
Comment at: llvm/test/CodeGen/ARM/ComplexArithmetic/complex-arithmetic-f32-add.ll:99
+  %b.imag = shufflevector <8 x float> %b, <8 x float> zeroinitializer, <4 x i32> <i32 1, i32 3, i32 5, i32 7>
+  %0 = fsub fast <4 x float> %b.real, %a.imag
+  %1 = fadd fast <4 x float> %b.imag, %a.real
----------------
dmgreen wrote:
> chill wrote:
> > chill wrote:
> > > Shouldn't these be translated  to a couple of `vcadd.f32` instructions, like in the previous test?
> > > And this amount of move instructions seems excessive.
> > > And this amount of move instructions seems excessive.
> > I guess MVE does not have sensible swizzling instructions.
> > 
> Yeah this is expected from shuffles that MVE cannot handle very well. It would look a lot better either with loads that could become interleaving loads, or under AArch64 where better shuffles are available.
> Shouldn't these be translated to a couple of vcadd.f32 instructions, like in the previous test?
Not in this case, no. The IR vectors are too wide to fit into actual vector registers, and the follow-up vector splitting patch (D129067) is restricted to splitting in half.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114174