Using -save-temps results in different code

Bug #687406 reported by Juha Kallioinen
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Linaro GCC
Fix Released
Low
Chung-Lin Tang
gcc
Fix Released
Medium

Bug Description

The attached example code produces different result when compiled with -save-temps than without.

I tested this with the gcc-4.5-arm-linux-gnueabi (4.5.1-7ubuntu1cross1.38) and also the latest Linaro November 2010 release.

This is the result with (linaro 2010.11 release):

$ arm-none-linux-gnueabi-gcc -Os -c mytest.c -o mytest-notemps.o
$ arm-none-linux-gnueabi-objdump -d mytest-notemps.o

mytest-notemps.o: file format elf32-littlearm

Disassembly of section .text:

00000000 <optimized>:
   0: e92d4800 push {fp, lr}
   4: e28db004 add fp, sp, #4
   8: e1a00000 nop ; (mov r0, r0)
   c: e8bd8800 pop {fp, pc}

...

$ arm-none-linux-gnueabi-gcc -save-temps -Os -c mytest.c -o mytest-savetemps.o
$ arm-none-linux-gnueabi-objdump -d mytest-savetemps.o

mytest-savetemps.o: file format elf32-littlearm

Disassembly of section .text:

00000000 <optimized>:
   0: e59f3020 ldr r3, [pc, #32] ; 28 <optimized+0x28>
   4: e92d4800 push {fp, lr}
   8: e28db004 add fp, sp, #4
   c: e5933000 ldr r3, [r3]
  10: e3130001 tst r3, #1
  14: 0a000001 beq 20 <optimized+0x20>
  18: e1a00000 nop ; (mov r0, r0)
  1c: e8bd8800 pop {fp, pc}
  20: e1a00000 nop ; (mov r0, r0)
  24: e8bd8800 pop {fp, pc}
  28: 00000000 .word 0x00000000
...

This gcc bug might be relevant: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38990

Related branches

Revision history for this message
Juha Kallioinen (juha-kallioinen) wrote :
Revision history for this message
Michael Hope (michaelh1) wrote :

Confirmed in gcc-linaro-2010.11-1 and upstream gcc 4.5.1.

Note that the example and particular usage are very unusual. The summary from the upstream bug is that you can get different results from multi-line #defines depending if you preprocess or not.

Changed in gcc-linaro:
importance: Undecided → Low
status: New → Triaged
Revision history for this message
Michael Hope (michaelh1) wrote :

Juha, as this isn't Linaro or ARM specific, could you please submit this upstream as well?

Revision history for this message
Chung-Lin Tang (cltang) wrote :

Looked at this a bit...

The situation is that, in GCC RTL, ASM statement RTXes (ASM_INPUT here, but may affect ASM_OPERANDS too) contain line number information as part of their fields.

The -save-temps option affect whether the source passes through the preprocessor; when it does (no -save-temps) the entire 'dummy' macro expands, and the compiler sees both asm statements as having the "same" location.

When it does not go through the preprocessor (-save-temps on, preprocessor runs and saves .i file, GCC then receives .i file and doesn't pass through preprocessor). Then the asm statements will be regarded as having "different" locations. Note that this places both functions in the test case (optimized() and non_optimized()) on the same basis.

When the ASM statements are regarded as "same location" (there internal location fields same), the crossjumping optimization code will then successfully combine both if-else clauses together, leaving only one ASM statement. When they are regarded as "different location"ed, the crossjumping optimization does not succeed.

Combine the location-vs-preprocessed issue, and the way crossjumping sees asm statements, and we see a difference in generated code induced by -save-temps. :P

I think I have a simple fix for this...

Revision history for this message
Juha Kallioinen (juha-kallioinen) wrote :

Thanks Tang for the explanation!

I can make an upstream bug about this too, I'll paste the link here once I've done it.

Since the bug is about preprocessing then this also has implications to distributed building like using distcc or icecc. I reported this problem originally because it was noticed that building linux kernel locally or with distcc results in these kinds of differences code. I'm told the kernel uses constructs like these somewhere in local_irq_restore().

Then it was also noticed that -save-temps causes the problem.

Revision history for this message
In , Juha Kallioinen (juha-kallioinen) wrote :

Created attachment 22691
example case

See the attached example case.

Also reported here:
https://bugs.launchpad.net/gcc-linaro/+bug/687406

When a piece of code containing a multiline macro construct with embedded asm code is first preprocessed then compiled into an object or asm file, the results are different than directly compiling them into an object file or asm code.

The preprocessor step could take place when using -save-temps. It also happens when using ccache or distcc.

Linux kernel arm parts have this kind of code at least in

local_irq_restore() [include/linux/irqflags.h]
  -> raw_local_irq_restore() [arch/arm/include/asm/irqflags.h]

----
Result of example case without -save-temps:

        .file "test.c"
        .text
.globl optimized
        .type optimized, @function
optimized:
.LFB0:
        .cfi_startproc
#APP
# 15 "/tmp/test.c" 1
        nop

# 0 "" 2
#NO_APP
        ret
        .cfi_endproc
.LFE0:
        .size optimized, .-optimized
        .ident "GCC: (GNU) 4.5.1"
        .section .note.GNU-stack,"",@progbits

----

Result of example case with -save-temps:

        .file "test.c"
        .text
.globl optimized
        .type optimized, @function
optimized:
.LFB0:
        .cfi_startproc
        testb $1, flags(%rip)
        je .L2
#APP
# 15 "/tmp/test.c" 1
        nop

# 0 "" 2
#NO_APP
        ret
.L2:
#APP
# 15 "/tmp/test.c" 1
        nop

# 0 "" 2
#NO_APP
        ret
        .cfi_endproc
.LFE0:
        .size optimized, .-optimized
        .ident "GCC: (GNU) 4.5.1"
        .section .note.GNU-stack,"",@progbits

Revision history for this message
Juha Kallioinen (juha-kallioinen) wrote :
Revision history for this message
In , Rguenth (rguenth) wrote :

Cross jumping doesn't see instructions with different location as equivalent
it seems. Comparing the 195r.csa dumps:

--- test(3).c.195r.csa 2010-12-09 15:41:13.000000000 +0100
+++ tmp/test(3).c.195r.csa 2010-12-09 15:39:56.000000000 +0100
@@ -11,19 +11,50 @@
 changing bb of uid 28
   from 5 to 4
 Merged 4 and 5 without moving.
+Cross jumping from bb 3 to bb 4; 1 common insns
+changing bb of uid 29
+ unscanned insn
+changing bb of uid 10
+ from 3 to 6
+changing bb of uid 26
+ from 3 to 6
+scanning new insn with uid = 30.
+deleting insn with uid = 29.
+deleting insn with uid = 10.
+deleting insn with uid = 26.
+deleting insn with uid = 22.
+deleting block 6

Thus, confirmed.

Revision history for this message
In , Rguenth (rguenth) wrote :

Works ok with 4.1.

Revision history for this message
In , Jakub-gcc (jakub-gcc) wrote :

Created attachment 22698
gcc46-pr46865.patch

The problem is that we do not guarantee the rtl locators are unique, locator_eq needs to be called to check equality.

Perhaps for 4.7 a better way would be to use a different fmt string letter for these locations and adjust all routines that look at rtl format strings.

Revision history for this message
In , Jakub-gcc (jakub-gcc) wrote :

Author: jakub
Date: Fri Dec 10 12:43:45 2010
New Revision: 167686

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=167686
Log:
 PR rtl-optimization/46865
 * rtl.c (rtx_equal_p_cb, rtx_equal_p): For last operand of
 ASM_OPERANDS and ASM_INPUT if integers are different,
 call locator_eq.
 * jump.c (rtx_renumbered_equal_p): Likewise.

 * gcc.target/i386/pr46865-1.c: New test.
 * gcc.target/i386/pr46865-2.c: New test.

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr46865-1.c
    trunk/gcc/testsuite/gcc.target/i386/pr46865-2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/jump.c
    trunk/gcc/rtl.c
    trunk/gcc/testsuite/ChangeLog

Revision history for this message
In , Jakub-gcc (jakub-gcc) wrote :

Fixed on the trunk so far.

Revision history for this message
In , Juha Kallioinen (juha-kallioinen) wrote :

Thanks for the quick work!

How does this get backported to 4.3 or 4.4 then? I can see the patch directly applies to 4.5.1, but not to 4.4.5 (rtl.c: rtx_equal_p is too different in it).

If this backporting stuff is something I should just know then don't waste your time explaining it in lenght, I can dig around :)

Christian Reis (kiko)
Changed in gcc:
importance: Undecided → Unknown
status: New → Unknown
Chung-Lin Tang (cltang)
Changed in gcc-linaro:
assignee: nobody → Chung-Lin Tang (cltang)
status: Triaged → In Progress
Michael Hope (michaelh1)
Changed in gcc-linaro:
milestone: none → 4.5-2011.01-0
status: In Progress → Fix Committed
Changed in gcc-linaro:
status: Fix Committed → Fix Released
Revision history for this message
In , Jakub-gcc (jakub-gcc) wrote :

Author: jakub
Date: Sun Jan 16 20:14:37 2011
New Revision: 168862

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168862
Log:
 Backport from mainline
 2010-12-10 Jakub Jelinek <email address hidden>

 PR rtl-optimization/46865
 * rtl.c (rtx_equal_p_cb, rtx_equal_p): For last operand of
 ASM_OPERANDS and ASM_INPUT if integers are different,
 call locator_eq.
 * jump.c (rtx_renumbered_equal_p): Likewise.

 * gcc.target/i386/pr46865-1.c: New test.
 * gcc.target/i386/pr46865-2.c: New test.

Added:
    branches/gcc-4_5-branch/gcc/testsuite/gcc.target/i386/pr46865-1.c
    branches/gcc-4_5-branch/gcc/testsuite/gcc.target/i386/pr46865-2.c
Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/jump.c
    branches/gcc-4_5-branch/gcc/rtl.c
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog

Revision history for this message
In , Jakub-gcc (jakub-gcc) wrote :

Author: jakub
Date: Sun Jan 16 22:53:09 2011
New Revision: 168877

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168877
Log:
 Backport from mainline
 2010-12-10 Jakub Jelinek <email address hidden>

 PR rtl-optimization/46865
 * rtl.c (rtx_equal_p_cb): For last operand of
 ASM_OPERANDS and ASM_INPUT if integers are different,
 call locator_eq.
 * jump.c (rtx_renumbered_equal_p): Likewise.

 * gcc.target/i386/pr46865-1.c: New test.
 * gcc.target/i386/pr46865-2.c: New test.

Added:
    branches/gcc-4_4-branch/gcc/testsuite/gcc.target/i386/pr46865-1.c
    branches/gcc-4_4-branch/gcc/testsuite/gcc.target/i386/pr46865-2.c
Modified:
    branches/gcc-4_4-branch/gcc/ChangeLog
    branches/gcc-4_4-branch/gcc/jump.c
    branches/gcc-4_4-branch/gcc/rtl.c
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog

Revision history for this message
In , Jakub-gcc (jakub-gcc) wrote :

Fixed for 4.4+.

Changed in gcc:
importance: Unknown → Medium
status: Unknown → In Progress
Revision history for this message
In , Christian Reis (kiko) wrote :

Is this bug now resolved, or is it pending any other backports?

Revision history for this message
In , Rguenth (rguenth) wrote :

Fixed for 4.4.6.

Changed in gcc:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Bug attachments

Remote bug watches

Bug watches keep track of this bug in other bug trackers.