[PATCH] D105020: [SLP]Improve graph reordering.

[Alexander Kornienko via Phabricator via llvm-commits]

alexfh added a comment.

In D105020#2999773 <https://reviews.llvm.org/D105020#2999773>, @ABataev wrote:

> In D105020#2999765 <https://reviews.llvm.org/D105020#2999765>, @xbolva00 wrote:
>
>> In D105020#2999752 <https://reviews.llvm.org/D105020#2999752>, @ABataev wrote:
>>
>>> In D105020#2975046 <https://reviews.llvm.org/D105020#2975046>, @goncharov wrote:
>>>
>>>> Here is a repro I found
>>>>
>>>>   $ cat ./repro.cc
>>>>   #include <iostream>
>>>>   namespace a {
>>>>   __attribute__((noinline)) void b(uint8_t *c, float *d, uint16_t e, uint16_t f) {
>>>>     uint16_t g = f;
>>>>     uint16_t h = c[0] << 2 | c[4] & 3;
>>>>     uint16_t l = c[1] << 2 | c[4] >> 2 & 3;
>>>>     uint16_t v = c[2] << 2 | 3;
>>>>     uint16_t j = c[3] << 2 | c[4] >> 6;
>>>>     *(d - 1) = float(l - e) / g;
>>>>     *(d - 2) = float(h - e) / g;
>>>>     *(d - 3) = float(j - e) / g;
>>>>     *(d - 4) = float(v - e) / g;
>>>>   }
>>>>   void bar(uint8_t *buffer, size_t k, size_t height, size_t, uint16_t e,
>>>>            uint16_t f, float *m) {
>>>>     int n = 24, g = f;
>>>>     float *t = &m[(24 - 2) * 4];
>>>>     size_t u = 2 * k;
>>>>     for (int o; o < height; o++) {
>>>>       uint8_t *p = buffer;
>>>>       uint8_t *q = p;
>>>>       float *r = &t[u];
>>>>       for (int a = 0; a < u; a += 4) {
>>>>         b(q, r, e, g);
>>>>         r -= 4;
>>>>       }
>>>>       t -= n;
>>>>     }
>>>>   }
>>>>   }  // namespace a
>>>>   int main() {
>>>>     uint8_t s[]{255, 0, 255, 0, 51};
>>>>     float m[4 * 24]{};
>>>>     a::bar(s, 4, 4, 0, 0, 1023, m);
>>>>     int *out = reinterpret_cast<int *>(m);
>>>>     int64_t sum;
>>>>     for (int i = 0; i < sizeof(m) / sizeof(int); i++) sum += out[i];
>>>>     std::cout << sum;
>>>>   }
>>>>   $ clang-before -O2 -std=gnu++17 repro.cc
>>>>   $ ./a.out
>>>>   17045651456
>>>>   $ clang-after -O2 -std=gnu++17 repro.cc
>>>>   $ ./a.out
>>>>   16609148640
>>>
>>> Sorry, the reproducer is not correct. It has not initialized variables, writes after array bounds etc. Unable to use it as a reproducer for the investigation.
>>
>> So if they are not able to produce UB-free reproducer, you should recommit the patch.
>
> I believe they have a reproducer and tried to reduce it but the tool just extracted/removed too much code here :). I'll try to check manually some parts of the code and wait for the actual reproducer, will recommit the patch in a couple of days if would not be able to find a bug/no correct reproducer provided.

Exactly, we reduced the code quite aggressively. We'll try to do this more carefully this time. Please hold on while we're on it. Last time it took multiple hours until we got something we could feed to creduce.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105020