HomePhabricator

[DAGCombine] Match a pattern where a wide type scalar value is stored by…

Description

[DAGCombine] Match a pattern where a wide type scalar value is stored by several narrow stores

This opportunity is found from spec 2017 557.xz_r. And it is used by the sha encrypt/decrypt. See sha-2/sha512.c

static void store64(u64 x, unsigned char* y)
{

for(int i = 0; i != 8; ++i)
    y[i] = (x >> ((7-i) * 8)) & 255;

}

static u64 load64(const unsigned char* y)
{

u64 res = 0;
for(int i = 0; i != 8; ++i)
    res |= (u64)(y[i]) << ((7-i) * 8);
return res;

}
The load64 has been implemented by https://reviews.llvm.org/D26149
This patch is trying to implement the store pattern.

Match a pattern where a wide type scalar value is stored by several narrow
stores. Fold it into a single store or a BSWAP and a store if the targets
supports it.

Assuming little endian target:
i8 *p = ...
i32 val = ...
p[0] = (val >> 0) & 0xFF;
p[1] = (val >> 8) & 0xFF;
p[2] = (val >> 16) & 0xFF;
p[3] = (val >> 24) & 0xFF;

*((i32)p) = val;

i8 *p = ...
i32 val = ...
p[0] = (val >> 24) & 0xFF;
p[1] = (val >> 16) & 0xFF;
p[2] = (val >> 8) & 0xFF;
p[3] = (val >> 0) & 0xFF;

*((i32)p) = BSWAP(val);

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

Event Timeline

This was identified as the culprit commit that broke the PPC big endian bot: http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/27963

Please pull the changeset and we can investigate the issue.
P.S. I am now seeing this issue on the little endian bot as well.

sbc100 added a subscriber: sbc100.Jun 4 2019, 1:31 PM

We also saw some strange breakage on the WebAssembly waterfall when this change was in the tree. The test that broke was https://github.com/gcc-mirror/gcc/blob/master/gcc/testsuite/gcc.c-torture/execute/920501-8.c. Although it was linked against musl libs so I imagine the breakage was deep in sprintf somewhere. Just FYI in case you indend to re-land.

I have created the patch https://reviews.llvm.org/D62897 to fix this issue. @sbc100 Do you have any step for me to reproduce the broken ? I tried the c code you mentioned, didn't see the difference. And it would be great if you could try the patch of https://reviews.llvm.org/D62897 together with this one to see, if the broken fixed.

sbc100 added a comment.Jun 5 2019, 3:13 PM

Yup, looks like D62897 fixes the issue I was seeing too!