[Bug target/66215] New: [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[Bug target/66215] New: [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[marxin at gcc dot gnu.org]

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

            Bug ID: 66215
           Summary: [4.8/4.9/5/6 Regression] Wrong after label NOP
                    emission for -mhotpatch
           Product: gcc
           Version: 6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: marxin at gcc dot gnu.org
  Target Milestone: ---
            Target: 390-linux

Hi.

Starting from r221381 GCC does not place nops right after function label.
Unfortunately, the problematic patch was also backported to gcc4-[89] and
gcc-5.

$ cat /tmp/s390.c 
static int foo()
{
  return 0;
}

int main(int argc, char **argv)
{
  return foo();
}

$ ./xgcc -B. /tmp/s390.c -mhotpatch=2,3 -o o0.s -S -fno-inline
$ cat o0.s | head ..

.text
        .align  8
        .type   foo, @function
        nopr    %r7     # pre-label NOPs for hotpatch (2 halfwords)
        nopr    %r7
        # alignment for hotpatch
        .align  8
foo:
        # post-label NOPs for hotpatch (3 halfwords)
.LFB0:
        stm     %r11,%r14,44(%r15)
.LCFI0:
        lr      %r11,%r15
.LCFI1:
        nop     0
        nopr    %r7
        lhi     %r1,0
        lr      %r2,%r1
        l       %r4,56(%r11)
        lm      %r11,%r14,44(%r11)
.LCFI2:
        br      %r4

Problem is that patched compiler relies that NOTE_INSN_FUNCTION_BEG is at the
beginning of function. 


/* Inject nops for hotpatching. */
+      for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
+       {
+         if (NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_FUNCTION_BEG)
+           break;
+       }

Problem is that if you use -O2, 256r.sched2 reorders insns after
pro_and_epilogue, where NOTE_INSN_FUNCTION_BEG is placed at the beginning.
On the other hand, if you try -O0, -O1, as the pass is not executed, emission
of NOTE_INSN_FUNCTION_BEG is not reordered.

$ cat s390.c.242r.pro_and_epilogue
foo

(note 1 0 5 NOTE_INSN_DELETED)
(note 5 1 18 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(insn/f 18 5 19 2 (set (mem:SI (plus:SI (reg/f:SI 15 %r15)
                (const_int 56 [0x38])) [1  S4 A8])
        (reg:SI 14 %r14)) /tmp/s390.c:7 -1
     (nil))
(insn 19 18 20 2 (set (reg:SI 5 %r5)
        (unspec_volatile [
                (const_int 0 [0])
            ] UNSPECV_MAIN_POOL)) /tmp/s390.c:7 -1
     (nil))
(note 20 19 17 2 NOTE_INSN_PROLOGUE_END)
(insn 17 20 4 2 (set (reg:SI 5 %r5)
        (unspec_volatile [
                (const_int 0 [0])
            ] UNSPECV_MAIN_POOL)) 675 {main_pool}
     (nil))
(note 4 17 8 2 NOTE_INSN_FUNCTION_BEG)
(insn 8 4 24 2 (set (reg:SI 1 %r1)
        (mem/u/c:SI (unspec:SI [
                    (symbol_ref/u:SI ("*.LC0") [flags 0x2])
                    (reg:SI 5 %r5)
                ] UNSPEC_LTREF) [2  S4 A32])) /tmp/s390.c:8 68 {*movsi_esa}
     (expr_list:REG_EQUAL (symbol_ref:SI ("foo") [flags 0x3]  <function_decl
0x7fc55997e3e0 foo>)
        (nil)))
(note 24 8 23 2 NOTE_INSN_EPILOGUE_BEG)
(insn/f 23 24 9 2 (set (reg:SI 14 %r14)
        (mem:SI (plus:SI (reg/f:SI 15 %r15)
                (const_int 56 [0x38])) [1  S4 A8])) /tmp/s390.c:9 -1
     (expr_list:REG_CFA_DEF_CFA (plus:SI (reg/f:SI 15 %r15)
            (const_int 96 [0x60]))
        (expr_list:REG_CFA_RESTORE (reg:SI 14 %r14)
            (nil))))
(call_insn/u/j 9 23 10 2 (set (reg:SI 2 %r2)
        (call (mem:QI (reg:SI 1 %r1) [0 foo S1 A8])
            (const_int 0 [0]))) /tmp/s390.c:8 631 {*sibcall_value_br}
     (expr_list:REG_CALL_DECL (symbol_ref:SI ("foo") [flags 0x3] 
<function_decl 0x7fc55997e3e0 foo>)
        (expr_list:REG_EH_REGION (const_int 0 [0])
            (nil)))
    (nil))
(barrier 10 9 16)
(note 16 10 0 NOTE_INSN_DELETED)


$ cat s390.c.256r.sched2
(note 1 0 3 NOTE_INSN_DELETED)
(note 3 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(note 2 3 15 2 NOTE_INSN_FUNCTION_BEG)
(insn/f 15 2 16 2 (set (mem:SI (plus:SI (reg/f:SI 15 %r15)
                (const_int 56 [0x38])) [1  S4 A8])
        (reg:SI 14 %r14)) /tmp/s390.c:2 68 {*movsi_esa}
     (expr_list:REG_DEAD (reg:SI 14 %r14)
        (nil)))
(insn 16 15 17 2 (set (reg:SI 5 %r5)
        (unspec_volatile [
                (const_int 0 [0])
            ] UNSPECV_MAIN_POOL)) /tmp/s390.c:2 675 {main_pool}
     (expr_list:REG_UNUSED (reg:SI 5 %r5)
        (nil)))
(note 17 16 14 2 NOTE_INSN_PROLOGUE_END)
(insn 14 17 25 2 (set (reg:SI 5 %r5)
        (unspec_volatile [
                (const_int 0 [0])
            ] UNSPECV_MAIN_POOL)) 675 {main_pool}
     (expr_list:REG_UNUSED (reg:SI 5 %r5)
        (nil)))
(note 25 14 21 2 NOTE_INSN_EPILOGUE_BEG)
(insn 21 25 9 2 (set (reg:SI 4 %r4)
        (mem:SI (plus:SI (reg/f:SI 15 %r15)
                (const_int 56 [0x38])) [1  S4 A8])) /tmp/s390.c:4 68
{*movsi_esa}
     (nil))
(insn 9 21 22 2 (set (reg/i:SI 2 %r2)
        (const_int 0 [0])) /tmp/s390.c:4 68 {*movsi_esa}
     (nil))
(insn/f 22 9 10 2 (set (reg:SI 14 %r14)
        (mem:SI (plus:SI (reg/f:SI 15 %r15)
                (const_int 56 [0x38])) [1  S4 A8])) /tmp/s390.c:4 68
{*movsi_esa}
     (expr_list:REG_CFA_DEF_CFA (plus:SI (reg/f:SI 15 %r15)
            (const_int 96 [0x60]))
        (expr_list:REG_CFA_RESTORE (reg:SI 14 %r14)
            (nil))))
(insn 10 22 23 2 (use (reg/i:SI 2 %r2)) /tmp/s390.c:4 -1
     (nil))
(jump_insn 23 10 24 2 (parallel [
            (return)
            (use (reg:SI 4 %r4))
        ]) /tmp/s390.c:4 681 {*return}
     (expr_list:REG_DEAD (reg:SI 4 %r4)
        (nil))
 -> return)

So the question is if NOTE_INSN_FUNCTION_BEG is not correctly located by
pro_and_epilogue, or we should fix NOP emission?

Thanks,
Martin

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[marxin at gcc dot gnu.org]

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

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Following testcase is wrongly compiled event with -O2 optimization level.

$ cat o2-test-case.c
static int a;

int t(int tt)
{
  switch (tt)
  {
    case 1: return a;
  }

  return 0;
}

$ ./xgcc -B. -mhotpatch=2,3 -O2 -fno-inline -o /tmp/test.s -S o2-test-case.c
$ head -n20 /tmp/test.s

        .file   "o2-test-case.c"
.text
        .align  8
.globl t
        .type   t, @function
        nopr    %r7     # pre-label NOPs for hotpatch (2 halfwords)
        nopr    %r7
        # alignment for hotpatch
        .align  8
t:
marxin@marxinbox:~/Programming/gcc2/objdir/gcc> head -n20 /tmp/test.s
        .file   "o2-test-case.c"
.text
        .align  8
.globl t
        .type   t, @function
        nopr    %r7     # pre-label NOPs for hotpatch (2 halfwords)
        nopr    %r7
        # alignment for hotpatch
        .align  8
t:
        # post-label NOPs for hotpatch (3 halfwords)
.LFB0:
        st      %r14,56(%r15)
        nop     0
        nopr    %r7
.LCFI0:
        l       %r4,56(%r15)
        lhi     %r2,0
        l       %r14,56(%r15)
.LCFI1:
>From gcc-bugs-return-486789-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Wed May 20 12:31:08 2015
Return-Path: <gcc-bugs-return-486789-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 14136 invoked by alias); 20 May 2015 12:31:08 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 10259 invoked by uid 48); 20 May 2015 12:31:04 -0000
From: "marxin at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
Date: Wed, 20 May 2015 12:31:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: target
X-Bugzilla-Version: 6.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: marxin at gcc dot gnu.org
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Resolution:
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-66215-4-dZyXcDwFbM@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-66215-4@http.gcc.gnu.org/bugzilla/>
References: <bug-66215-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2015-05/txt/msg01629.txt.bz2
Content-length: 1204

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

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Martin Liška from comment #1)
> Following testcase is wrongly compiled event with -O2 optimization level.
> 
> $ cat o2-test-case.c
> static int a;
> 
> int t(int tt)
> {
>   switch (tt)
>   {
>     case 1: return a;
>   }
> 
>   return 0;
> }
> 
> $ ./xgcc -B. -mhotpatch=2,3 -O2 -fno-inline -o /tmp/test.s -S o2-test-case.c
> $ head -n20 /tmp/test.s
> 
> 	.file	"o2-test-case.c"
> .text
> 	.align	8
> .globl t
> 	.type	t, @function
> 	nopr	%r7	# pre-label NOPs for hotpatch (2 halfwords)
> 	nopr	%r7
> 	# alignment for hotpatch
> 	.align	8
> t:
> marxin@marxinbox:~/Programming/gcc2/objdir/gcc> head -n20 /tmp/test.s
> 	.file	"o2-test-case.c"
> .text
> 	.align	8
> .globl t
> 	.type	t, @function
> 	nopr	%r7	# pre-label NOPs for hotpatch (2 halfwords)
> 	nopr	%r7
> 	# alignment for hotpatch
> 	.align	8
> t:
> 	# post-label NOPs for hotpatch (3 halfwords)
> .LFB0:
> 	st	%r14,56(%r15)
> 	nop	0
> 	nopr	%r7
> .LCFI0:
> 	l	%r4,56(%r15)
> 	lhi	%r2,0
> 	l	%r14,56(%r15)
> .LCFI1:

Sorry, this testcase is invalid as I modified the compiler.

Martin
>From gcc-bugs-return-486791-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Wed May 20 12:33:55 2015
Return-Path: <gcc-bugs-return-486791-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 21397 invoked by alias); 20 May 2015 12:33:55 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 21345 invoked by uid 48); 20 May 2015 12:33:51 -0000
From: "krebbel at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch
Date: Wed, 20 May 2015 12:33:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: target
X-Bugzilla-Version: 6.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: krebbel at gcc dot gnu.org
X-Bugzilla-Status: NEW
X-Bugzilla-Resolution:
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: bug_status cf_reconfirmed_on everconfirmed
Message-ID: <bug-66215-4-5E9ogEEIl7@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-66215-4@http.gcc.gnu.org/bugzilla/>
References: <bug-66215-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2015-05/txt/msg01631.txt.bz2
Content-length: 404

https://gcc.gnu.org/bugzilla/show_bug.cgi?idf215

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-05-20
     Ever confirmed|0                           |1

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
IMHO the nops should go immediately before the first real instruction in the
function.  The point of not emitting it earlier is so that the nops are already
covered by debug and unwind info.
So, we certainly should skip all NOTEs at the start of the function, and
perhaps
selected other insns (dunno if e.g. UNSPECV_MAIN_POOL insn emits any code or
not).  NOTE_INSN_FUNCTION_BEG is not a good place, as it can be preceeded by
various prologue instructions, and the nops really have to go before all real
instructions (that emit some bytes into the function text section).

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[krebbel at gcc dot gnu.org]

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

--- Comment #4 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> IMHO the nops should go immediately before the first real instruction in the
> function.  The point of not emitting it earlier is so that the nops are
> already covered by debug and unwind info.
> So, we certainly should skip all NOTEs at the start of the function, and
> perhaps
> selected other insns (dunno if e.g. UNSPECV_MAIN_POOL insn emits any code or
> not).  NOTE_INSN_FUNCTION_BEG is not a good place, as it can be preceeded by
> various prologue instructions, and the nops really have to go before all
> real instructions (that emit some bytes into the function text section).

UNSPECV_MAIN_POOL does not itself emit code. However, it is used as an anchor
to emit insns right after it so we should not skip it here I think.

We will try with skipping all the NOTEs as you suggested and we definitely
should run all the hotpatch testcases at different optimization levels.

Thanks!

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[vogt at linux dot vnet.ibm.com]

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

--- Comment #5 from Dominik Vogt <vogt at linux dot vnet.ibm.com> ---
Wouldn't the correct and easy to identify place be right after the first
NOTE_INSN_BASIC_BLOCK?

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
No, IMHO you can have many debug insns after that and before first real insn.
I'd go for something like:
  rtx_insn *insn = get_insns ();
  if (!active_insn_p (insn))
    insn = next_active_insn (insn);
and insert before, rather than after (otherwise you don't handle the
hypothetical case of an active insn being the first one).
That would require rewriting the nop insertion code after it, because you want
to insert the 6 byte nops first.  Or just gcc_assert the first insn is not
active, or if the first insn is active, emit a NOTE_INSN_DELETED note before
that
first active insn, emit the nops after that note and perhaps kill the note at
the end.
Please test
void foo (void) { __builtin_unreachable (); }
actually generates any active insns.

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.8.5

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[vogt at linux dot vnet.ibm.com]

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

--- Comment #7 from Dominik Vogt <vogt at linux dot vnet.ibm.com> ---
Created attachment 35595
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35595&action=edit
Experimental fix

Can you please check whether this patch works for you?  The patch also contains
improved test cases that should hopefully detect anything like this happening
again.

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Doesn't this disable all the other s390 tests?
Supposedly you need to prune them from the non-torture lists, see e.g.
g++.dg/dg.exp on how it prunes tests.

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[vogt at linux dot vnet.ibm.com]

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

--- Comment #9 from Dominik Vogt <vogt at linux dot vnet.ibm.com> ---
Yeah, I accidentally left the other test cases commented out.  I'll clean that
up in the next version of the patch.  That doesn't have an impact on the
correctness of the code change, though.

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[jakub at gcc dot gnu.org]

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
As for the hotpatch testcases, have you tested them also with
-fno-dwarf2-cfi-asm (as they seem to contain .cfi directives in the regexps)?
And for the main patch, the change will revert the order of 2, 4 and 6 byte
nops post-label, is that ok?  Say if some hotpatching code were to verify it
overwrites the nops and required the old order.

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[vogt at linux dot vnet.ibm.com]

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

--- Comment #11 from Dominik Vogt <vogt at linux dot vnet.ibm.com> ---
(In reply to Jakub Jelinek from comment #10)
> As for the hotpatch testcases, have you tested them also with
> -fno-dwarf2-cfi-asm (as they seem to contain .cfi directives in the regexps)?

Not yet.  I'll add another test case for that.  The regexp is supposed to skip
all lines with .cfi_... or labels matching .L*:, so -fno-dwarf2-cfi-asm should
not matter, but testing is besser than guessing.

> And for the main patch, the change will revert the order of 2, 4 and 6 byte
> nops post-label

The order is the same because the patch never updates "insn".  The order would
be reversed if I had written "insn = emit_before_insn (...)", keeping the
assignment.  The tests indirectly verify that the order is correct.  For
example, hotpatch-5.c tests that with -mhotpatch=...,4 the first instruction
after the comment is a 6-byte-NOP:

hp1:
        # post-label NOPs for hotpatch (4 halfwords)
.LFB0:
        .cfi_startproc
        brcl    0, 0
        nopr    %r7
        stmg    %r11,%r15,88(%r15)

> is that ok?

No, that would be incorrect.  It is important that the largest NOP comes first

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[vogt at linux dot vnet.ibm.com]

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

--- Comment #12 from Dominik Vogt <vogt at linux dot vnet.ibm.com> ---
Created attachment 35597
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35597&action=edit
Experimental fix 2

Version 2 of the patch with the suggested changes and test fixes.

Note: Functions without active insns will get the pre-label NOPs, the comment
"# post-label NOPs" but no NOPs after that.  Not sure whether it's worth the
effort to clean this up.

Please test if this fixes the observed problem (it does for me).

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[jakub at gcc dot gnu.org]

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

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The only function without active insns I'm aware of is:
void
foo (void)
{
  __builtin_unreachable ();
}
which only contains notes and a BARRIER, nothing else (at least on s390x).
It would be desirable to handle even that, otherwise a hotpatcher would
overwrite random bytes from some other function.
One option would be to avoid using next_active_insn:
for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
  if (active_insn_p (insn) || BARRIER_P (insn))
    break;
Adding the nops after a BARRIER is generally undesirable I'd say.

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[vogt at linux dot vnet.ibm.com]

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

--- Comment #14 from Dominik Vogt <vogt at linux dot vnet.ibm.com> ---
Is it possible that a function has no insns at all?

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[vogt at linux dot vnet.ibm.com]

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

Dominik Vogt <vogt at linux dot vnet.ibm.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #35597|0                           |1
        is obsolete|                            |

--- Comment #15 from Dominik Vogt <vogt at linux dot vnet.ibm.com> ---
Created attachment 35598
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35598&action=edit
Experimental fix 3

Version 3 of the patch with the suggested change; also fixes th
egcc_unreachable test.

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[jakub at gcc dot gnu.org]

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

--- Comment #16 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The function I've mentioned has:
(note 1 0 3 NOTE_INSN_DELETED)
(note 3 1 12 [bb 2] NOTE_INSN_BASIC_BLOCK)
(note 12 3 2 NOTE_INSN_PROLOGUE_END)
(note 2 12 5 NOTE_INSN_FUNCTION_BEG)
(barrier 5 2 8)
(note 8 5 0 NOTE_INSN_DELETED)
(at least in my somewhat older trunk version).
I can't think of anything simpler than that, because if the function isn't
noreturn, it shall have at least a return insn at the end, and if it is
noreturn, but doesn't have __builtin_unreachable () in it, there should be
either an endless loop (so some jump_insn), or noreturn call (call_insn).
Though, thinking about it, if you only have notes at the beginning and then
some CODE_LABEL, adding the nops after the CODE_LABEL would result in running
the nops not just when entering the function the first time, but more often.
Thus, I'd suggest maybe:
for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
  if (active_insn_p (insn)
      || BARRIER_P (insn)
      || LABEL_P (insn))
    break;
Thus, put the nops before first of active insns, barrier (that one for the
__builtin_unreachable () case) and the last one for e.g.:
void
baz (volatile unsigned int *i)
{
  for (;;)
    (*i)++;
}
where I guess hotpatching wouldn't really like to see this as running the
function many times.

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[vogt at linux dot vnet.ibm.com]

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

Dominik Vogt <vogt at linux dot vnet.ibm.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #35598|0                           |1
        is obsolete|                            |

--- Comment #17 from Dominik Vogt <vogt at linux dot vnet.ibm.com> ---
Created attachment 35599
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35599&action=edit
Experimental fix 4

Version 4 of the patch with the suggested change and a test case for it.

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[mbenes at suse dot cz]

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

--- Comment #18 from Miroslav Benes <mbenes at suse dot cz> ---
I confirm that v4 of the proposed patch works for me. Tested on simple
userspace program similar to the one in the bug description and on the kernel
module where I stumbled upon the bug originally.

Thanks a lot for the fix,
Miroslav

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[jakub at gcc dot gnu.org]

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

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I'm still missing (untested):
-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cS\]]] \
+dg-runtest [lsort [prune [glob -nocomplain $srcdir/$subdir/*.\[cS\]] \
+                         $srcdir/$subdir/hotpatch-\[0-9\]*.c]] \
         "" $DEFAULT_CFLAGS
or similar change in the s390.exp, otherwise you run all the hotpatch-[0-9]*.c
tests once outside of the torture (with the implicit -O0) and many times in the
torture mode (where it includes -O0 too).

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[vogt at linux dot vnet.ibm.com]

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

--- Comment #20 from Dominik Vogt <vogt at linux dot vnet.ibm.com> ---
I'll remove "-O0" from the list of torture test options so that the list of
hotpatch torture tests is only defined in one place.

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[jakub at gcc dot gnu.org]

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

--- Comment #21 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
In case somebody forces any of the torture options into the default through
--target_board=unix/WHATEVER that will result in testing some option
combination multiple times and -O0 never.  If you want to avoid duplication,
you can just
set hotpatch_tests $srcdir/$subdir/hotpatch-\[0-9\]*.c
and use $hotpatch_tests in both cases (untested).

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[vogt at linux dot vnet.ibm.com]

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

Dominik Vogt <vogt at linux dot vnet.ibm.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #35599|0                           |1
        is obsolete|                            |

--- Comment #22 from Dominik Vogt <vogt at linux dot vnet.ibm.com> ---
Created attachment 35628
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35628&action=edit
Experimental fix 5

Version 5 with the suggested changes and a new test case.  Hopefully the last
version before submitting the changes on gcc-patches.

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[jakub at gcc dot gnu.org]

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

--- Comment #23 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Dominik Vogt from comment #22)
> Created attachment 35628 [details]
> Experimental fix 5
> 
> Version 5 with the suggested changes and a new test case.  Hopefully the
> last version before submitting the changes on gcc-patches.

+dg-runtest [lsort [prune [glob -nocomplain $srcdir/$subdir/hotpatch-*.\[cS\]]
\
+                        $hotpatch_tests]] "" $DEFAULT_CFLAGS

is wrong, that will run all hotpatch-*.[cS] tests that are not
hotpatch-[0-9].[cS] (i.e. just hotpatch-compile*, but not all the other s390
tests).

You want (IMHO):

+dg-runtest [lsort [prune [glob -nocomplain $srcdir/$subdir/*.\[cS\]] \
+                        $hotpatch_tests]] "" $DEFAULT_CFLAGS

instead.

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[vogt at linux dot vnet.ibm.com]

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

--- Comment #24 from Dominik Vogt <vogt at linux dot vnet.ibm.com> ---
Okay, I've made a new patch that fixes the problem with only the hotpatch tests
running in the s390 test suite.  This change passes all tests on 64 and 31 bit.
 After this change we have done some cleanup work:

* Added a comment to the "if (active_insn_p (insn) ...)",
* removed the now redundant -fno-dwarf2-cfi-asm from the torture tests,
* removed the changes from the hopatch-compile*.c tests as they are not going
through the torture tests anyway,
* updated the ChangeLog.

The new set of changes is being tested right now, but just with the upstream
code, without bootstrapping and without the Ada testsuite.  This appears to be
acceptable to me given that the latest changes don't affect the code.

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[vogt at linux dot vnet.ibm.com]

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

--- Comment #26 from Dominik Vogt <vogt at linux dot vnet.ibm.com> ---
The patch for upstream gcc is available here:
https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02739.html

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[krebbel at gcc dot gnu.org]

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

--- Comment #25 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
The patch looks good to me. Please post it on the mailing list and I'll commit
it. 

Jakub, thanks for your valuable comments! Does Dominik's last patch address
your concerns?

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[krebbel at gcc dot gnu.org]

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

--- Comment #27 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
Author: krebbel
Date: Fri May 29 14:26:56 2015
New Revision: 223867

URL: https://gcc.gnu.org/viewcvs?rev=223867&root=gcc&view=rev
Log:
PR 66215: S390: Fix placement of post-label NOPs with -mhotpatch

gcc/ChangeLog:
            PR target/66215
            * config/s390/s390.c (s390_reorg): Fix placement of post-label NOPs
            with -mhotpatch=.

gcc/testsuite/ChangeLog:
            PR target/66215
            * gcc.target/s390/hotpatch-1.c: Remove optimization options from
            dg-options.
            * gcc.target/s390/hotpatch-10.c: Likewise.
            * gcc.target/s390/hotpatch-11.c: Likewise.
            * gcc.target/s390/hotpatch-12.c: Likewise.
            * gcc.target/s390/hotpatch-17.c: Likewise.
            * gcc.target/s390/hotpatch-18.c: Likewise.
            * gcc.target/s390/hotpatch-20.c: Likewise.
            * gcc.target/s390/hotpatch-21.c: Likewise.
            * gcc.target/s390/hotpatch-22.c: Likewise.
            * gcc.target/s390/hotpatch-23.c: Likewise.
            * gcc.target/s390/hotpatch-24.c: Likewise.
            * gcc.target/s390/hotpatch-2.c: Likewise.  Adjust scan-assembler
            to check for the exact nops too.
            * gcc.target/s390/hotpatch-3.c: Likewise.
            * gcc.target/s390/hotpatch-4.c: Likewise.
            * gcc.target/s390/hotpatch-5.c: Likewise.
            * gcc.target/s390/hotpatch-6.c: Likewise.
            * gcc.target/s390/hotpatch-7.c: Likewise.
            * gcc.target/s390/hotpatch-8.c: Likewise.
            * gcc.target/s390/hotpatch-9.c: Likewise.
            * gcc.target/s390/hotpatch-14.c: Likewise.
            * gcc.target/s390/hotpatch-15.c: Likewise.
            * gcc.target/s390/hotpatch-16.c: Likewise.
            * gcc.target/s390/hotpatch-19.c: Likewise.
            * gcc.target/s390/hotpatch-25.c: Likewise.  Remove
            scan-assembler-times counting number of .align directives.
            * gcc.target/s390/hotpatch-13.c: Remove optimization options from
            dg-options.  Remove scan-assembler-times counting number of .align
            directives.
            * gcc.target/s390/hotpatch-26.c: New file.
            * gcc.target/s390/hotpatch-27.c: New file.
            * gcc.target/s390/hotpatch-28.c: New file.
            * gcc.target/s390/s390.exp: Run hotpatch-*.c tests as torture tests
            using -Os -O0 -O1 -O2 -O3 options.


Added:
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-26.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-27.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-28.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/s390/s390.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-1.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-10.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-11.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-12.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-13.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-14.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-15.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-16.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-17.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-18.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-19.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-2.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-20.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-21.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-22.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-23.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-24.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-25.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-3.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-4.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-5.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-6.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-7.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-8.c
    trunk/gcc/testsuite/gcc.target/s390/hotpatch-9.c
    trunk/gcc/testsuite/gcc.target/s390/s390.exp

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[jakub at gcc dot gnu.org]

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

--- Comment #28 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
BTW, the hotpatch-19.c, hotpatch-20.c and hotpatch-compile-15.c testcases look
wrong, always_inline attribute shouldn't be used on functions not declared
inline.

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[vogt at linux dot vnet.ibm.com]

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

--- Comment #29 from Dominik Vogt <vogt at linux dot vnet.ibm.com> ---
As this still seems to work in 4.8, 4.9 and 5.1, is it acceptable to only fix
this on the master?

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[jakub at gcc dot gnu.org]

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

--- Comment #30 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
IMHO it is better to keep the tests in sync between branches.

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[krebbel at gcc dot gnu.org]

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

--- Comment #31 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
Author: krebbel
Date: Mon Jun  1 11:25:56 2015
New Revision: 223930

URL: https://gcc.gnu.org/viewcvs?rev=223930&root=gcc&view=rev
Log:
PR 66215: S390: Fix placement of post-label NOPs with -mhotpatch

gcc/ChangeLog:
--------------
2015-06-01  Dominik Vogt  <vogt@linux.vnet.ibm.com>

            Backport from mainline
            2015-05-29  Dominik Vogt  <vogt@linux.vnet.ibm.com>

            PR target/66215
            * config/s390/s390.c (s390_reorg): Fix placement of post-label NOPs
            with -mhotpatch=.

gcc/testsuite/ChangeLog:
------------------------
2015-06-01  Dominik Vogt  <vogt@linux.vnet.ibm.com>

            Backport from mainline
            2015-05-29  Dominik Vogt  <vogt@linux.vnet.ibm.com>

            PR target/66215
            * gcc.target/s390/hotpatch-1.c: Remove optimization options from
            dg-options.
            * gcc.target/s390/hotpatch-10.c: Likewise.
            * gcc.target/s390/hotpatch-11.c: Likewise.
            * gcc.target/s390/hotpatch-12.c: Likewise.
            * gcc.target/s390/hotpatch-17.c: Likewise.
            * gcc.target/s390/hotpatch-18.c: Likewise.
            * gcc.target/s390/hotpatch-20.c: Likewise.
            * gcc.target/s390/hotpatch-21.c: Likewise.
            * gcc.target/s390/hotpatch-22.c: Likewise.
            * gcc.target/s390/hotpatch-23.c: Likewise.
            * gcc.target/s390/hotpatch-24.c: Likewise.
            * gcc.target/s390/hotpatch-2.c: Likewise.  Adjust scan-assembler
            to check for the exact nops too.
            * gcc.target/s390/hotpatch-3.c: Likewise.
            * gcc.target/s390/hotpatch-4.c: Likewise.
            * gcc.target/s390/hotpatch-5.c: Likewise.
            * gcc.target/s390/hotpatch-6.c: Likewise.
            * gcc.target/s390/hotpatch-7.c: Likewise.
            * gcc.target/s390/hotpatch-8.c: Likewise.
            * gcc.target/s390/hotpatch-9.c: Likewise.
            * gcc.target/s390/hotpatch-14.c: Likewise.
            * gcc.target/s390/hotpatch-15.c: Likewise.
            * gcc.target/s390/hotpatch-16.c: Likewise.
            * gcc.target/s390/hotpatch-19.c: Likewise.
            * gcc.target/s390/hotpatch-25.c: Likewise.  Remove
            scan-assembler-times counting number of .align directives.
            * gcc.target/s390/hotpatch-13.c: Remove optimization options from
            dg-options.  Remove scan-assembler-times counting number of .align
            directives.
            * gcc.target/s390/hotpatch-26.c: New file.
            * gcc.target/s390/hotpatch-27.c: New file.
            * gcc.target/s390/hotpatch-28.c: New file.
            * gcc.target/s390/s390.exp: Run hotpatch-*.c tests as torture tests
            using -Os -O0 -O1 -O2 -O3 options.


Added:
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-26.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-27.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-28.c
Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/config/s390/s390.c
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-1.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-10.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-11.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-12.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-13.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-14.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-15.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-16.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-17.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-18.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-19.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-2.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-20.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-21.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-22.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-23.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-24.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-25.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-3.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-4.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-5.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-6.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-7.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-8.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/hotpatch-9.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/s390/s390.exp

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[krebbel at gcc dot gnu.org]

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

--- Comment #32 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
Author: krebbel
Date: Mon Jun  1 11:28:09 2015
New Revision: 223931

URL: https://gcc.gnu.org/viewcvs?rev=223931&root=gcc&view=rev
Log:
PR 66215: S390: Fix placement of post-label NOPs with -mhotpatch

gcc/ChangeLog:
--------------
2015-06-01  Dominik Vogt  <vogt@linux.vnet.ibm.com>

            Backport from mainline
            2015-05-29  Dominik Vogt  <vogt@linux.vnet.ibm.com>

            PR target/66215
            * config/s390/s390.c (s390_reorg): Fix placement of post-label NOPs
            with -mhotpatch=.

gcc/testsuite/ChangeLog:
------------------------
2015-06-01  Dominik Vogt  <vogt@linux.vnet.ibm.com>

            Backport from mainline
            2015-05-29  Dominik Vogt  <vogt@linux.vnet.ibm.com>

            PR target/66215
            * gcc.target/s390/hotpatch-1.c: Remove optimization options from
            dg-options.
            * gcc.target/s390/hotpatch-10.c: Likewise.
            * gcc.target/s390/hotpatch-11.c: Likewise.
            * gcc.target/s390/hotpatch-12.c: Likewise.
            * gcc.target/s390/hotpatch-17.c: Likewise.
            * gcc.target/s390/hotpatch-18.c: Likewise.
            * gcc.target/s390/hotpatch-20.c: Likewise.
            * gcc.target/s390/hotpatch-21.c: Likewise.
            * gcc.target/s390/hotpatch-22.c: Likewise.
            * gcc.target/s390/hotpatch-23.c: Likewise.
            * gcc.target/s390/hotpatch-24.c: Likewise.
            * gcc.target/s390/hotpatch-2.c: Likewise.  Adjust scan-assembler
            to check for the exact nops too.
            * gcc.target/s390/hotpatch-3.c: Likewise.
            * gcc.target/s390/hotpatch-4.c: Likewise.
            * gcc.target/s390/hotpatch-5.c: Likewise.
            * gcc.target/s390/hotpatch-6.c: Likewise.
            * gcc.target/s390/hotpatch-7.c: Likewise.
            * gcc.target/s390/hotpatch-8.c: Likewise.
            * gcc.target/s390/hotpatch-9.c: Likewise.
            * gcc.target/s390/hotpatch-14.c: Likewise.
            * gcc.target/s390/hotpatch-15.c: Likewise.
            * gcc.target/s390/hotpatch-16.c: Likewise.
            * gcc.target/s390/hotpatch-19.c: Likewise.
            * gcc.target/s390/hotpatch-25.c: Likewise.  Remove
            scan-assembler-times counting number of .align directives.
            * gcc.target/s390/hotpatch-13.c: Remove optimization options from
            dg-options.  Remove scan-assembler-times counting number of .align
            directives.
            * gcc.target/s390/hotpatch-26.c: New file.
            * gcc.target/s390/hotpatch-27.c: New file.
            * gcc.target/s390/hotpatch-28.c: New file.
            * gcc.target/s390/s390.exp: Run hotpatch-*.c tests as torture tests
            using -Os -O0 -O1 -O2 -O3 options.


Added:
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-26.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-27.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-28.c
Modified:
    branches/gcc-4_9-branch/gcc/ChangeLog
    branches/gcc-4_9-branch/gcc/config/s390/s390.c
    branches/gcc-4_9-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-1.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-10.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-11.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-12.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-13.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-14.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-15.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-16.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-17.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-18.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-19.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-2.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-20.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-21.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-22.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-23.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-24.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-25.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-3.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-4.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-5.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-6.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-7.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-8.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/hotpatch-9.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.target/s390/s390.exp

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[krebbel at gcc dot gnu.org]

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

--- Comment #33 from Andreas Krebbel <krebbel at gcc dot gnu.org> ---
Author: krebbel
Date: Mon Jun  1 11:29:46 2015
New Revision: 223932

URL: https://gcc.gnu.org/viewcvs?rev=223932&root=gcc&view=rev
Log:
PR 66215: S390: Fix placement of post-label NOPs with -mhotpatch

gcc/ChangeLog:
--------------
2015-06-01  Dominik Vogt  <vogt@linux.vnet.ibm.com>

            Backport from mainline
            2015-05-29  Dominik Vogt  <vogt@linux.vnet.ibm.com>

            PR target/66215
            * config/s390/s390.c (s390_reorg): Fix placement of post-label NOPs
            with -mhotpatch=.

gcc/testsuite/ChangeLog:
------------------------
2015-06-01  Dominik Vogt  <vogt@linux.vnet.ibm.com>

            Backport from mainline
            2015-05-29  Dominik Vogt  <vogt@linux.vnet.ibm.com>

            PR target/66215
            * gcc.target/s390/hotpatch-1.c: Remove optimization options from
            dg-options.
            * gcc.target/s390/hotpatch-10.c: Likewise.
            * gcc.target/s390/hotpatch-11.c: Likewise.
            * gcc.target/s390/hotpatch-12.c: Likewise.
            * gcc.target/s390/hotpatch-17.c: Likewise.
            * gcc.target/s390/hotpatch-18.c: Likewise.
            * gcc.target/s390/hotpatch-20.c: Likewise.
            * gcc.target/s390/hotpatch-21.c: Likewise.
            * gcc.target/s390/hotpatch-22.c: Likewise.
            * gcc.target/s390/hotpatch-23.c: Likewise.
            * gcc.target/s390/hotpatch-24.c: Likewise.
            * gcc.target/s390/hotpatch-2.c: Likewise.  Adjust scan-assembler
            to check for the exact nops too.
            * gcc.target/s390/hotpatch-3.c: Likewise.
            * gcc.target/s390/hotpatch-4.c: Likewise.
            * gcc.target/s390/hotpatch-5.c: Likewise.
            * gcc.target/s390/hotpatch-6.c: Likewise.
            * gcc.target/s390/hotpatch-7.c: Likewise.
            * gcc.target/s390/hotpatch-8.c: Likewise.
            * gcc.target/s390/hotpatch-9.c: Likewise.
            * gcc.target/s390/hotpatch-14.c: Likewise.
            * gcc.target/s390/hotpatch-15.c: Likewise.
            * gcc.target/s390/hotpatch-16.c: Likewise.
            * gcc.target/s390/hotpatch-19.c: Likewise.
            * gcc.target/s390/hotpatch-25.c: Likewise.  Remove
            scan-assembler-times counting number of .align directives.
            * gcc.target/s390/hotpatch-13.c: Remove optimization options from
            dg-options.  Remove scan-assembler-times counting number of .align
            directives.
            * gcc.target/s390/hotpatch-26.c: New file.
            * gcc.target/s390/hotpatch-27.c: New file.
            * gcc.target/s390/hotpatch-28.c: New file.
            * gcc.target/s390/s390.exp: Run hotpatch-*.c tests as torture tests
            using -Os -O0 -O1 -O2 -O3 options.


Added:
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-26.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-27.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-28.c
Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/config/s390/s390.c
    branches/gcc-5-branch/gcc/testsuite/ChangeLog
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-1.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-10.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-11.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-12.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-13.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-14.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-15.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-16.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-17.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-18.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-19.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-2.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-20.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-21.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-22.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-23.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-24.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-25.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-3.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-4.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-5.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-6.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-7.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-8.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/hotpatch-9.c
    branches/gcc-5-branch/gcc/testsuite/gcc.target/s390/s390.exp

[Bug target/66215] [4.8/4.9/5/6 Regression] Wrong after label NOP emission for -mhotpatch

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

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

--- Comment #34 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.