misoptimization with defaulting to POWER7 and newer

[pkubaj]

Bugzilla Link	45709
Resolution	FIXED
Resolved on	Jun 22, 2020 15:47
Version	trunk
OS	FreeBSD
Blocks	#44654
CC	@adalava,@DimitryAndric,@MaskRay,@nemanjai,@tstellar
Fixed by commit(s)	793cc51 eac91d5

Extended Description

Reduced test case:
class a {
void b();
void m_fn2();
int c;
float d[];
};
void a ::b() {
float e;
for (int f; f <= c; f++)
if (e < d[f])
e = d[f];
if (e)
m_fn2();
}

Compile with:
c++ -ffast-math -O2 -c sms.cpp

It builds fine without -ffast-math, or when lowering optimizations to -O1, or after adding -mcpu=power7 (power8 or power9 are also ok for clang).

Result:
Illegal instruction

[pkubaj]

assigned to @hfinkel

[pkubaj]

This is on LLVM 10.0.0 on FreeBSD head, powerpc64 elfv2.

[adalava]

the behavior is a bit different for me, tested on POWER8 machine:

1 - builds fine with llvm 9.0.1
2 - on llvm 10.0.0 compiler gets stuck and doesn't return (I've canceled it after ~2 minutes)

[pkubaj]

the behavior is a bit different for me, tested on POWER8 machine:

1 - builds fine with llvm 9.0.1
2 - on llvm 10.0.0 compiler gets stuck and doesn't return (I've canceled it
after ~2 minutes)

That's interesting. I find that the failed builds take much longer than the passing ones (e.g. with -mcpu=power7), but it still doesn't take THAT much:
pkubaj@talos:$/usr/home/pkubaj/audacity$ time c++ -ffast-math -O2 -c sms.cpp
Illegal instruction (core dumped)
0m29.39s real 0m01.26s user 0m27.71s system

Can you truss the clang++ process to see what it does?

[adalava]

Can you truss the clang++ process to see what it does?

Nevermind, there was high load in the data storage and time was being consumed writing core file. I can reproduce sigill too.

LLVM 9.0.1 - ok
LLVM 10.0.0 - sigill
LLVM 11 - ok

It looks like a regression that was already fixed in recent versions. Need to check 10.0.1.

[MaskRay]

Can you truss the clang++ process to see what it does?

Nevermind, there was high load in the data storage and time was being
consumed writing core file. I can reproduce sigill too.

LLVM 9.0.1 - ok
LLVM 10.0.0 - sigill
LLVM 11 - ok

It looks like a regression that was already fixed in recent versions. Need
to check 10.0.1.

I confirmed this is fixed at HEAD. Alfredo, can you figure out which patch fixes the bug? I think we'd like to include it for the 10.0.1 minor release.

Note that rc1 will happen soon http://lists.llvm.org/pipermail/llvm-dev/2020-April/141128.html

[adalava]

I confirmed this is fixed at HEAD. Alfredo, can you figure out which patch
fixes the bug? I think we'd like to include it for the 10.0.1 minor release.

Fangrui, I identified problem was introduced by the following commit:

c3d05c1 [SelectionDAG] Expand nnan FMINNUM/FMAXNUM to select sequence

The reduced test case builds fine with this patch reverted.

Now I'm trying to identify which commit(s) fixed that on HEAD.

[adalava]

Bisect points "d6f47aeb5198b142072d8ce2bbf2fdd30d116db0 [SCEV] SCEVExpander::isHighCostExpansionHelper(): cost-model min/max (#44668 )" as the commit that fixed the problem in llvm 11.

I tried to cherry pick it to releases/10.x branch. but there is merge conflict. Need to understand the context and if other patches need to be backported as well.

[pkubaj]

Could you merge that to 10.0.1?

[tstellar]

Alfredo, are you still working on generating a patch that applies cleanly or would you like me to help with this?

[adalava]

Alfredo, are you still working on generating a patch that applies cleanly or
would you like me to help with this?

Hi Tom,
I started studying the code but I haven't yet being able to continue.

I don't wan't delay the release and you're experienced in this code, so I'd be happy if you could help, for sure!

Thanks!

[tstellar]

Alfredo, are you still working on generating a patch that applies cleanly or
would you like me to help with this?

Hi Tom,
I started studying the code but I haven't yet being able to continue.

I don't wan't delay the release and you're experienced in this code, so I'd
be happy if you could help, for sure!

Thanks!

Ok, I will give it a try.

[tstellar]

The commit that fixed the problem appears to be part of a large series of improvements that is going to be hard to backport. However, I think the commit is just hiding the bug and not actually fixing it.

I have been unable to reproduce this bug, would someone be able to post a stack trace of the compiler when it crashes as well as the output from this command:

clang++ -S -emit-llvm -ffast-math -O2 -c sms.cpp -o -

[tstellar]

I was just able to reproduce by specifying -mcpu=power6:

clang++ -mcpu=power6 -ffast-math -O2 -c sms.cpp

[tstellar]

Reduced LLVM IR testcase
Reproduce with: llc < pr45709-reduced.ll