On 12/1/20 7:09 PM, Ilya Leoshkevich wrote:
> On Tue, 2020-12-01 at 15:34 -0700, Jeff Law wrote:
>> No strong opinions. I think whichever is less invasive in terms of
>> code
>> quality is probably the way to go. What we want to avoid is
>> suppressing
>> threading unnecessarily as that often leads to false positives from
>> middle-end based warnings. Suppressing threading can also lead to
>> build
>> failures in the kernel due to the way they use b_c_p.
> I think v1 is better then. Would you mind approving the following?
> That's the same code as in v1, but with the improved commit message and
> comments.
>
>
>
> Linux Kernel (specifically, drivers/leds/trigger/ledtrig-cpu.c) build
> with GCC 10 fails on s390 with "impossible constraint".
>
> Explanation by Jeff Law:
>
> ```
> So what we have is a b_c_p at the start of an if-else chain. Subsequent
> tests on the "true" arm of the the b_c_p test may throw us off the
> constant path (because the constants are out of range). Once all the
> tests are passed (it's constant and the constant is in range) the true
> arm's terminal block has a special asm that requires a constant
> argument. In the case where we get to the terminal block on the true
> arm, the argument to the b_c_p is used as the constant argument to the
> special asm.
>
> At first glace jump threading seems to be doing the right thing. Except
> that we end up with two paths to that terminal block with the special
> asm, one for each of the two constant arguments to the b_c_p call.
> Naturally since that same value is used in the asm, we have to introduce
> a PHI to select between them at the head of the terminal block. Now
> the argument in the asm is no longer constant and boom we fail.
> ```
>
> Fix by disallowing __builtin_constant_p on threading paths.
>
> gcc/ChangeLog:
>
> 2020-06-03 Ilya Leoshkevich <i...@linux.ibm.com>
>
> * tree-ssa-threadbackward.c (thread_jumps::profitable_jump_thread_path):
> Do not allow __builtin_constant_p on a threading path.
>
> gcc/testsuite/ChangeLog:
>
> 2020-06-03 Ilya Leoshkevich <i...@linux.ibm.com>
>
> * gcc.target/s390/builtin-constant-p-threading.c: New test.
OK. I think the old forward threader has the same problem. Which I
think can be fixed by returning NULL from
record_temporary_equivalences_from_stmts_at_dest when we see the B_C_P
call. Fixing that in the obvious way is pre-approved once it's gone
through the usual testing.
jeff
