[PATCH] D100495: [SLP] Add detection of shuffled/perfect matching of tree entries.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

In D100495#2715441 <https://reviews.llvm.org/D100495#2715441>, @phawkins wrote:

> In D100495#2713228 <https://reviews.llvm.org/D100495#2713228>, @ABataev wrote:
>
>> In D100495#2711644 <https://reviews.llvm.org/D100495#2711644>, @bkramer wrote:
>>
>>> Seeing a miscompile after this has landed. Trying to reduce a test case, but no luck so far.
>>
>> Any updates about a reproducer?
>
> Here's the smallest reproducer we have so far.
>
> Grab:
>
> - driver.cc: https://gist.github.com/hawkinsp/de3ee1adc72d9e4618e00cde5139a5c3
> - buffer-assignment.txt: https://gist.github.com/hawkinsp/01719b73adb20cccb8d06a57267d42a3
> - module.ll: https://gist.github.com/hawkinsp/cbe6e3d508f6bf8172d29bd0c6f986a0
>
> and run (on a machine that supports AVX 512):
>
>   llvm-project/build/bin/clang  module.ll  driver.cc -o repro -march=skylake-avx512 -O3 -lstdc++ -lm
>   ./repro buffer-assignment.txt
>
> Before this PR (at the preceding commit) this repro produces:
>
>   Output:
>   0.502001, -0.462127, 0.459155, 0.10126, -0.0533019, -0.557239
>
> After this PR, this repro produces:
>
>   Output:
>   0.519662, -0.450291, 0.492509, 0.0910437, -0.0563238, -0.522649
>
> We expect this procedure to be accurate to something more like 1e-10, so this is a significant difference.

Thanks for the reproducer again, investigated it. This patch is not a real cause of the issue, it just revealed again a known problem with masked gathers. Just after this patch, we started to vectorize more code with gathers. If I completely disable masked gathers in the code, the result I get is the next:

  0.502001, -0.462127, 0.459155, 0.10126, -0.0533019, -0.557239

. And looks like it is correct. @anton-afanasyev, maybe there is a bug in lowering of masked gathers? And we really need to check if it is profitable/allowed to use masked gathers. I have initial patch for it in the non-power-2 patch, will prepare it as a separate patch later today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100495