[Bug testsuite/64796] New: effective target bswap64 globally caches target-specific use of lp64

[Bug testsuite/64796] New: effective target bswap64 globally caches target-specific use of lp64

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64796

            Bug ID: 64796
           Summary: effective target bswap64 globally caches
                    target-specific use of lp64
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: trivial
          Priority: P3
         Component: testsuite
          Assignee: unassigned at gcc dot gnu.org
          Reporter: vries at gcc dot gnu.org

1. unix/: supported

$ make -k -j5 -d check-gcc 'RUNTESTFLAGS=dg.exp=optimize-bswapdi-1.c -v -v
--target_board=unix/'
...
Schedule of variations:
    unix/

Running target unix/
Running /home/vries/gcc_versions/devel/master/src/gcc/testsuite/gcc.dg/dg.exp
PASS: gcc.dg/optimize-bswapdi-1.c (test for excess errors)
PASS: gcc.dg/optimize-bswapdi-1.c scan-tree-dump-times bswap "64 bit bswap
implementation found at" 3
...


2. unix/-m32: unsupported

$ make -k -j5 -d check-gcc 'RUNTESTFLAGS=dg.exp=optimize-bswapdi-1.c -v -v
--target_board=unix/-m32':
...
Schedule of variations:
    unix/-m32

Running target unix/-m32
Running /home/vries/gcc_versions/devel/master/src/gcc/testsuite/gcc.dg/dg.exp
UNSUPPORTED: gcc.dg/optimize-bswapdi-1.c
...


3. unix/ unix/-m32: supported
$ make -k -j5 -d check-gcc 'RUNTESTFLAGS=dg.exp=optimize-bswapdi-1.c -v -v
--target_board=unix/\ unix/-m32'
...
Schedule of variations:
    unix/
    unix/-m32

Running target unix/
Running /home/vries/gcc_versions/devel/master/src/gcc/testsuite/gcc.dg/dg.exp 
PASS: gcc.dg/optimize-bswapdi-1.c (test for excess errors)
PASS: gcc.dg/optimize-bswapdi-1.c scan-tree-dump-times bswap "64 bit bswap
implementation found at" 3

Running target unix/-m32
Running /home/vries/gcc_versions/devel/master/src/gcc/testsuite/gcc.dg/dg.exp 
PASS: gcc.dg/optimize-bswapdi-1.c (test for excess errors)
PASS: gcc.dg/optimize-bswapdi-1.c scan-tree-dump-times bswap "64 bit bswap
implementation found at" 3
...


The bswap64 effective target is cached globally, so it caches the
target-specific use of lp64:
...
 Return 1 if the target supports 64-bit byte swap instructions.                 

proc check_effective_target_bswap64 { } {
    global et_bswap64_saved

    if [info exists et_bswap64_saved] {
        verbose "check_effective_target_bswap64: using cached result" 2
    } else {
        set et_bswap64_saved 0
        if { [is-effective-target bswap]
             && [is-effective-target lp64] } {
           set et_bswap64_saved 1
        }
    }

    verbose "check_effective_target_bswap64: returning $et_bswap64_saved" 2
    return $et_bswap64_saved
}
...

Note that the test passes with -m32, so maybe the effective target bswap64 is
not needed, or the wrong one.

[Bug testsuite/64796] effective target bswap64 globally caches target-specific use of lp64

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64796

vries at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |thomas.preudhomme at arm dot com

--- Comment #1 from vries at gcc dot gnu.org ---
cc-ing author

[Bug testsuite/64796] effective target bswap64 globally caches target-specific use of lp64

[thopre01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64796

thopre01 at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2015-01-27
                 CC|                            |thopre01 at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |thopre01 at gcc dot gnu.org
     Ever confirmed|0                           |1
      Known to fail|                            |5.0

--- Comment #2 from thopre01 at gcc dot gnu.org ---
Indeed, I didn't realize --target_board could specifies several boards, which
rules out the use of a global. It is also true that 32-bit targets can do a
64-bit byte swap thanks to expand_doubleword_bswap () (called from expand_unop
when GET_MODE_SIZE (mode) == 2 * UNITS_PER_WORD and that there is a bswap optab
for word_mode).

Testing a patch right now.

[Bug testsuite/64796] effective target bswap64 globally caches target-specific use of lp64

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64796

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hjl.tools at gmail dot com

--- Comment #3 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to vries from comment #0)

> proc check_effective_target_bswap64 { } {
>     global et_bswap64_saved
> 
>     if [info exists et_bswap64_saved] {
>         verbose "check_effective_target_bswap64: using cached result" 2
>     } else {
>         set et_bswap64_saved 0
>         if { [is-effective-target bswap]
>              && [is-effective-target lp64] } {
>            set et_bswap64_saved 1
>         }
>     }
> 
>     verbose "check_effective_target_bswap64: returning $et_bswap64_saved" 2
>     return $et_bswap64_saved
> }

That is wrong for x32.  X32 is ILP32 and should support bswap64.

[Bug testsuite/64796] effective target bswap64 globally caches target-specific use of lp64

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64796

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to H.J. Lu from comment #3) 
> That is wrong for x32.  X32 is ILP32 and should support bswap64.

And wrong for MIPS N32 and even AARCH64 ILP32.  We need to special case those
three targets.

Even wrong for PowerPC with -mpowerpc64 enabled too.

[Bug testsuite/64796] effective target bswap64 globally caches target-specific use of lp64

[thopre01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64796

--- Comment #5 from Thomas Preud'homme <thopre01 at gcc dot gnu.org> ---
These cases should all be covered by the patch posted [1], shouldn't it?

[1] https://gcc.gnu.org/ml/gcc-patches/2015-01/msg02380.html

[Bug testsuite/64796] effective target bswap64 globally caches target-specific use of lp64

[thopre01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64796

--- Comment #6 from Thomas Preud'homme <thopre01 at gcc dot gnu.org> ---
Author: thopre01
Date: Wed Feb  4 01:54:47 2015
New Revision: 220388

URL: https://gcc.gnu.org/viewcvs?rev=220388&root=gcc&view=rev
Log:
2015-02-04  Thomas Preud'homme  <thomas.preudhomme@arm.com>

    PR testsuite/64796
    * lib/target-supports.exp (check_effective_target_bswap64): Do not
    cache result in a global variable.  Include all 32-bit targets for
    bswap64 tests.

Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/lib/target-supports.exp

[Bug testsuite/64796] effective target bswap64 globally caches target-specific use of lp64

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64796

--- Comment #7 from vries at gcc dot gnu.org ---
Fix was committed. marking resolved, fixed

[Bug testsuite/64796] effective target bswap64 globally caches target-specific use of lp64

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64796

vries at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from vries at gcc dot gnu.org ---
As per previous comment