HomePhabricator

[InstCombine] matchFunnelShift - support non-uniform constant vector shift…

Authored by RKSimon on Oct 8 2020, 4:56 AM.

Description

[InstCombine] matchFunnelShift - support non-uniform constant vector shift amounts (PR46895)

Complete basic PR46895 fixes by refactoring D87452/D88402 to allow us to match non-uniform constant values.

We still don't handle non-uniform vectors that contain undef elements, but that can wait until we have a decent generic mechanism for this.

Differential Revision: https://reviews.llvm.org/D88420

Event Timeline

The miscompiled function is probably:

typedef uint64_t crypto_word_t;
static void poly2_lshift1(struct poly2 *p) {
  crypto_word_t carry = 0;
  for (size_t i = 0; i < ((701 + (sizeof(crypto_word_t) * 8) - 1) / (sizeof(crypto_word_t) * 8)); i++) {
    const crypto_word_t next_carry = p->v[i] >> ((sizeof(crypto_word_t) * 8) - 1);
    p->v[i] <<= 1;
    p->v[i] |= carry;
    carry = next_carry;
  }
}

With optimization on, it gets inlined so it is kind of hard to extract the code. But attribute(optnone) on this function does enable the test to pass. I suppose there may be others.

I'm working on a simple case with input and outputs to test.

This comment was removed by saugustine.

I spoke too soon. Disabling optimization on that function does fix the problem, but its input and output is all the same. So the miscompile must just not get tickled by that. Will dig a little deeper.

@saugustine we probably should move this to a bugzilla ticket if that is OK - is there any way that you can get the ir output with/without the non-uniform constant matchers in matchShiftAmount and get the diff? It should be something to do with vector rotate handling in the dag - either generic or powerpc specific - but I'd like to see the ir diff to help narrow it down.