llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.84k stars 11.47k forks source link

[Instcombine] Excessive folding causes isel to crash #55209

Open JonPsson opened 2 years ago

JonPsson commented 2 years ago

testcase.tar.gz

clang -O3 -target s390x-linux-gnu -march=arch13 crash3_aftercreduce.c -o a.out -mllvm -unroll-allow-remainder=false -mllvm -unroll-count=10

After the unrolling, there are a very great number of xor instructions, but instcombiner combines them all into a collossall xor with xor expressions, which seems to be too much for SelectionDAGBuilder.

Maybe there should be a limit to how many folded expressions instcombine produces, so that this does not become a problem?

nikic commented 2 years ago

From the InstCombine side, everything looks fine to me here. InstCombine just produces a large constant expression -- it's not even an interesting one (linear rather than exponential). If SDAGBuilder cannot handle it, it needs to be fixed on that side. It works fine for me (when you say "too much", do you mean you get a stack overflow?)

PS: This is part of the reason why the concept of constant expressions needs to be removed.

JonPsson commented 2 years ago

Yes, I believe it was a stack overflow...

I just thought maybe it wouldn't make sense to take e.g. 10k instructions and fold them all into one and then do isel. It sounds more like a stress test to me :-) The simplest thing to me seems to avoid this in the first place. I might argue that it is unreasonable and that if there is no benefit we shouldn't rewrite ISel algorithms if it's simple to avoid... But that's just my first thoughts...

xgupta commented 1 year ago

Is the issue still reproduced on trunk, seems not after 15 - https://godbolt.org/z/s7GcEdbP3.

nikic commented 1 year ago

This probably requires setting a smaller stack size to reproduce. This probably still exists, though there is also a pretty clear way forward now: Remove the xor constant expression per https://discourse.llvm.org/t/rfc-remove-most-constant-expressions/63179. Someone just needs to do the work for that.