[PATCH] tree-ssa-threadbackward.c (profitable_jump_thread_path): Do not allow __builtin_constant_p.

[Ilya Leoshkevich]

On Fri, 2020-06-26 at 14:27 -0600, Jeff Law wrote:
> On Thu, 2020-06-04 at 02:37 +0200, Ilya Leoshkevich via Gcc-patches
> wrote:
> > Bootstrapped and regtested on x86_64-redhat-linux, ppc64le-redhat-
> > linux
> > and s390x-redhat-linux.
> > 
> > 
> > Linux Kernel (specifically, drivers/leds/trigger/ledtrig-cpu.c)
> > build
> > with GCC 10 fails on s390 with "impossible constraint".
> > 
> > The problem is that jump threading makes __builtin_constant_p lie
> > when
> > an expression in question appears to be a constant on a threading
> > path.
> What do you mean by this?
> 
> What should happen is the path where the queried object is constant,
> builtin_constant_p will return true.  On the other path where it is
> not,
> builtin_constant_p will return false.

But what if a path ends before we use the `builtin_constant_p` result?
In the testcase from the patch we have `i = is_active ? 1 : -1`, which 
produces two paths.  On each path `builtin_constant_p (i)` returns 
true, however, the asm statement which depends on this being true is
not on either path and comes only later.

Here are the relevant tree dumps (sorry, a bit verbose, I couldn't make
them smaller):

gcc/cc1 ../gcc/testsuite/gcc.target/s390/builtin-constant-p-threading.c 
-O2 -march=z196 -mzarch -fdump-tree-all-all

Before 106t.thread1:

;;   basic block 2
  if (is_active_2(D) != 0)
    goto <bb 3>; [50.00%]
  else
    goto <bb 4>; [50.00%]

;;   basic block 3
  # iftmp.0_6 = PHI <1(2)>
  _7 = __builtin_constant_pD.1098 (iftmp.0_6);
  if (_7 != 0)
    goto <bb 6>; [50.00%]
  else
    goto <bb 8>; [50.00%]

;;   basic block 4
  # iftmp.0_4 = PHI <-1(2)>
  _12 = __builtin_constant_pD.1098 (iftmp.0_4);
  if (_12 != 0)
    goto <bb 5>; [50.00%]
  else
    goto <bb 8>; [50.00%]

;;   basic block 5
  if (iftmp.0_4 >= -128)
    goto <bb 7>; [20.00%]
  else
    goto <bb 8>; [80.00%]

;;   basic block 6
  if (iftmp.0_6 <= 127)
    goto <bb 7>; [12.00%]
  else
    goto <bb 8>; [88.00%]

;;   basic block 7
  # iftmp.0_13 = PHI <iftmp.0_6(6), iftmp.0_4(5)>
  # .MEM_10 = VDEF <.MEM_3(D)>
  __asm__ __volatile__("asi %0,%1
" : "ptr" "=Q" MEM[(intD.6 *)&num_active_cpusD.2277] : "val" "i"
iftmp.0_13, "Q" MEM[(intD.6 *)&num_active_cpusD.2277] : "memory",
"cc");
  goto <bb 9>; [100.00%]
;;    succ:       9 [always]  count:91268056 (estimated locally)
(FALLTHRU,EXECUTABLE)

;;   basic block 8
  # iftmp.0_14 = PHI <iftmp.0_4(5), iftmp.0_4(4), iftmp.0_6(6),
iftmp.0_6(3)>
  # .MEM_11 = VDEF <.MEM_3(D)>
  __asm__ __volatile__("laa %0,%2,%1
" : "old" "=d" old_8, "ptr" "=Q" MEM[(intD.6 *)&num_active_cpusD.2277]
: "val" "d" iftmp.0_14, "Q" MEM[(intD.6 *)&num_active_cpusD.2277] :
"memory", "cc");

`asi` instruction (bb 7) can be used only with constants, and that's 
what `__builtin_constant_p` ensures here.  For other values `laa`
instruction (block 8) must be used.

106t.thread1 says:

about to thread: path: 2 -> 4, 4 -> 5, 5 -> 6
about to thread: path: 3 -> 4, 4 -> 5, 5 -> 6, 6 -> 7

and as a result produces this:

;;   basic block 2
  if (is_active_2(D) != 0)
    goto <bb 3>; [50.00%]
  else
    goto <bb 4>; [50.00%]

;;   basic block 3

;;   basic block 4
  # iftmp.0_13 = PHI <-1(2), 1(3)>
  # .MEM_10 = VDEF <.MEM_3(D)>
  __asm__ __volatile__("asi %0,%1
" : "ptr" "=Q" MEM[(intD.6 *)&num_active_cpusD.2277] : "val" "i"
iftmp.0_13, "Q" MEM[(intD.6 *)&num_active_cpusD.2277] : "memory",
"cc");

This code won't compile - `iftmp.0_13` is not a constant.  My idea was
that the root cause is that `i` is constant on each threading path,
but is not constant anymore when they merge.  But maybe there is a
deeper issue?  What would be a better way to fix this?

> > Fix by disallowing __builtin_constant_p on threading paths.
> > 
> > gcc/ChangeLog:
> > 
> > 2020-06-03  Ilya Leoshkevich  <iii@linux.ibm.com>
> > 
> > 	* tree-ssa-threadbackward.c
> > (thread_jumps::profitable_jump_thread_path):
> > 	Do not allow __builtin_constant_p on a threading path.
> Sorry, this is wrong.
> 
> jeff
> 
>