[PATCH] D46008: [X86][AArch64][NFC] Add tests for vector masked merge unfolding

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun May 20 09:04:09 PDT 2018


lebedev.ri added a comment.

Thank you for the review!

In https://reviews.llvm.org/D46008#1105896, @spatel wrote:

> In https://reviews.llvm.org/D46008#1101250, @lebedev.ri wrote:
>
> > One observation i can make - under `SSE1`, the `v8i32` are scalarized, not split into 2x `v4i32`.
> >  It looks like a bug?
>
>
> I think that's the same thing I mentioned in https://reviews.llvm.org/D46528. We're not transforming to x86-specific opcodes early, so legalization does the safe thing for the illegal types - scalarization. Nobody cares about the perf of SSE1-only at this point, so it's not worth fixing IMO. That also means it's not worth testing so thoroughly here (and I think even x86 fans' eyeballs would fall out before they get anywhere close to confirming those entire asm sequences are correct!).




> So I'd second Simon's opinion that we just remove the SSE1 run, but if you insist...LGTM. :)

Yeah, i'm conflicted.
I agree that it is not of too much use, but then why do we have separate `hasSSE1()` and `hasSSE2()` knobs? :)
And if they are separate, and the tests clearly indicate that it matters here...

So i'll let this sit here for a bit longer, and wait for @RKSimon to re-confirm that they should be dropped.


Repository:
  rL LLVM

https://reviews.llvm.org/D46008





More information about the llvm-commits mailing list