[Bug bootstrap/51860] New: [4.7 regression] s390 esa mode bootstrap comparison failure since transactional memory branch

[Bug bootstrap/51860] New: [4.7 regression] s390 esa mode bootstrap comparison failure since transactional memory branch

[krebbel at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51860

             Bug #: 51860
           Summary: [4.7 regression] s390 esa mode bootstrap comparison
                    failure since transactional memory branch
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: critical
          Priority: P3
         Component: bootstrap
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: krebbel@gcc.gnu.org


Since revision r181154 the ESA mode bootstrap fails on s390 with a bootstrap
comparison failure (the daily built builds with --with-mode=zarch).

The c_common_nodes_and_builtins function does seem to have a different size
comparing the two stages.

stage2: 83336 c_common_nodes_and_builtins()
stage3: 83528 c_common_nodes_and_builtins()

I have a closer look next week.

[Bug bootstrap/51860] [4.7 regression] s390 esa mode bootstrap comparison failure since transactional memory branch merge

[krebbel at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51860

Andreas Krebbel <krebbel at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |s390-ibm-linux
           Priority|P3                          |P1
               Host|                            |s390-ibm-linux
            Summary|[4.7 regression] s390 esa   |[4.7 regression] s390 esa
                   |mode bootstrap comparison   |mode bootstrap comparison
                   |failure since transactional |failure since transactional
                   |memory branch               |memory branch merge
              Build|                            |s390-ibm-linux

[Bug bootstrap/51860] [4.7 regression] s390 esa mode bootstrap comparison failure since transactional memory branch merge

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51860

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-01-15
                 CC|                            |jakub at gcc dot gnu.org
   Target Milestone|---                         |4.7.0
     Ever Confirmed|0                           |1

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-15 19:04:00 UTC ---
I can reproduce it with
CC='gcc -m31' CXX='g++ -m31' s390 ../configure --enable-languages=c,c++,fortran
--with-mode=esa

[Bug bootstrap/51860] [4.7 regression] s390 esa mode bootstrap comparison failure since transactional memory branch merge

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51860

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-15 20:25:20 UTC ---
Reducing a -fcompare-debug failure on c-common.c with delta.

[Bug bootstrap/51860] [4.7 regression] s390 esa mode bootstrap comparison failure since transactional memory branch merge

[krebbel at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51860

--- Comment #3 from Andreas Krebbel <krebbel at gcc dot gnu.org> 2012-01-15 20:37:02 UTC ---
(In reply to comment #2)
> Reducing a -fcompare-debug failure on c-common.c with delta.

Thanks for working on this! I already had a quick look. The problem has to do
with the s390 reorg pass having to split the literal pool. The jump over the
literal pool is placed differently with and without debug info.

There are several places where we still use NEXT_INSN or INSN_P what will be
affected by DEBUG_INSNs being present. I'll track this down tomorrow.

no debug info:

L33: <bb: 48  size: 14  insns: 5>
        0x4ff9fc  basr    r13, r0                         
        0x4ff9fe  la      r13, 14(r13)                    
        0x4ffa02  basr    r14, r12                        
        0x4ffa04  basr    r14, r11                        
        0x4ffa06  j       L34                             
<bb: 49  size: 2  insns: 1>
        0x4ffa0a  nopr    r7                              
L34: <bb: 50  size: 80  insns: 22>
        0x4ffb60  lhi     r1, 0
        0x4ffb64  icm     r1, 3, 0(r2)  

with debug info:

L33: <bb: 48  size: 18  insns: 6>
        0x4ff9fc  basr    r13, r0                         
        0x4ff9fe  la      r13, 18(r13)                    
        0x4ffa02  basr    r14, r12                        
        0x4ffa04  basr    r14, r11                        
        0x4ffa06  lhi     r1, 0                           
        0x4ffa0a  j       L34                             
<bb: 49  size: 2  insns: 1>
        0x4ffa0e  nopr    r7                              
L34: <bb: 50  size: 76  insns: 21>
        0x4ffb64  icm     r1, 3, 0(r2)

[Bug bootstrap/51860] [4.7 regression] s390 esa mode bootstrap comparison failure since transactional memory branch merge

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51860

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-15 21:01:23 UTC ---
Well, DEBUG_INSNs aren't in the stream during the reorg anymore, it is about
the NOTE_INSN_VAR_LOCATION and NOTE_INSN_CALL_ARG_LOCATION (apparently the
latter in this testcase) in the stream that make the chunkify code insert it at
different spots.

[Bug bootstrap/51860] [4.7 regression] s390 esa mode bootstrap comparison failure since transactional memory branch merge

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51860

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-16 00:06:26 UTC ---
Created attachment 26335
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26335
pr51860.ii

Partly reduced testcase.
g++ -O2 -mesa -fcompare-debug pr51860.ii
Unfortunately, delta is quite slow to reduce it even further than this (most
probably because it depends on the inter insn distances to hit the literal pool
insertion at the exact spot).

[Bug bootstrap/51860] [4.7 regression] s390 esa mode bootstrap comparison failure since transactional memory branch merge

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51860

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-16 07:17:58 UTC ---
Created attachment 26336
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26336
patch

I believe the problem is that INSN_ADDRESSES of the CALL_INSN is different from
INSN_ADDRESSES of the NOTE_INSN_CALL_ARG_LOCATION that follows it (which is
equal to the next insn).
Will look at this in more detail shortly.

[Bug bootstrap/51860] [4.7 regression] s390 esa mode bootstrap comparison failure since transactional memory branch merge

[krebbel at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51860

--- Comment #7 from Andreas Krebbel <krebbel at gcc dot gnu.org> 2012-01-16 08:23:56 UTC ---
Regarding the insn address the split is supposed to happen after the call_insn
3059.

But the code I've added to keep the call_arg_location note after the call insn
prevents that:
      /* Make sure we do not split between a call and its
         corresponding CALL_ARG_LOCATION note.  */
      if (CALL_P (insn))
        {
          rtx next = NEXT_INSN (insn);
          if (next && NOTE_P (next)
          && NOTE_KIND (next) == NOTE_INSN_CALL_ARG_LOCATION)
        continue;
        }

Then the split should occur just after the call_arg_location note but that code
prevents that:

/* We can insert the barrier only after a 'real' insn.  */
  if (GET_CODE (insn) != INSN && GET_CODE (insn) != CALL_INSN)
    continue;

So the split in fact occurs after the next real insn which is the load zero.

(call_insn 3059 39956 39958 (parallel [
            (set (reg:SI 2 %r2)
                (call (mem:QI (reg/f:SI 11 %r11 [15205]) [0
identifier_global_value S1 A8])
                    (const_int 0 [0])))
            (clobber (reg:SI 14 %r14))
        ]) /build/gcc-head/gcc/c-family/c-common.c:5045 576 {*basr_r}
     (nil)
    (expr_list:REG_CFA_RESTORE (use (reg:SI 2 %r2))
        (nil)))

(note 39958 3059 37799 (nil) NOTE_INSN_CALL_ARG_LOCATION)

(insn 37799 39958 44088 (set (reg:SI 1 %r1 [5752])
        (const_int 0 [0])) /build/gcc-head/gcc/c-family/c-common.c:5045 67
{*movsi_esa}
     (nil))

(jump_insn 44088 37799 44089 (set (pc)
        (label_ref 44090)) /build/gcc-head/gcc/c-family/c-common.c:5045 561
{*jump31}
     (nil)

[Bug bootstrap/51860] [4.7 regression] s390 esa mode bootstrap comparison failure since transactional memory branch merge

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51860

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #26336|0                           |1
        is obsolete|                            |
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |jakub at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-16 10:56:39 UTC ---
Created attachment 26337
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26337
gcc47-pr51860.patch

Updated patch.  The previous patch still failed on this testcase with
-fcompare-debug failure, while the insns were the same, the jump had different
INSN_LOCATOR, because once it has been taken from the call insn and once from
the note.

[Bug bootstrap/51860] [4.7 regression] s390 esa mode bootstrap comparison failure since transactional memory branch merge

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51860

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-16 13:21:09 UTC ---
The second patch bootstrapped with
CC='gcc -m31' CXX='g++ -m31' s390 ../configure --enable-languages=c,c++,fortran
--with-mode=esa
Running make check now.

[Bug bootstrap/51860] [4.7 regression] s390 esa mode bootstrap comparison failure since transactional memory branch merge

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51860

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-16 14:05:59 UTC ---
Author: jakub
Date: Mon Jan 16 14:05:54 2012
New Revision: 183210

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=183210
Log:
    PR bootstrap/51860
    * config/s390/s390.c (s390_chunkify_start): Don't skip
    call insns followed by NOTE_INSN_CALL_ARG_LOCATION note.  Skip
    NOTE_INSN_VAR_LOCATION and NOTE_INSN_CALL_ARG_LOCATION notes.
    If insn is followed by NOTE_INSN_VAR_LOCATION or
    NOTE_INSN_CALL_ARG_LOCATION notes, insert jump after all those notes.
    Don't use location of note insns.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/s390/s390.c

[Bug bootstrap/51860] [4.7 regression] s390 esa mode bootstrap comparison failure since transactional memory branch merge

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51860

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-01-16 14:10:01 UTC ---
Fixed.