Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GCC: ARITHMODE restore not dominated by save #53

Open
margnus1 opened this issue Jul 14, 2015 · 1 comment
Open

GCC: ARITHMODE restore not dominated by save #53

margnus1 opened this issue Jul 14, 2015 · 1 comment

Comments

@margnus1
Copy link

Hi! I believe I have stumbled upon a bug in your GCC backend:

#define FUNNY_NUMBER1  268440163

/*
 * When compiled with -Os, -Og or -O0 and sz==0, I will set the ARITHMODE flags
 * of the config register from an unitilialized gp register.
 */
unsigned
hash(char *ptr, unsigned sz, unsigned hash)
{
    while (sz--) {
        hash = hash*FUNNY_NUMBER1 + *ptr++;
    }
    return hash;
}

The issue is easily spotted in the assembly output:

kalle ~/repro> e-gcc -Og -S -c gcc.c && cat gcc.s
    .file   "gcc.c"
    .section .text
    .balign 4
    .global _hash
_hash:
    mov r3,r0
    mov r0,r2
    mov r17, %low(#268440163)
    movt r17, %high(#268440163)
    b .L2
.L3:
    mov r1, %low(#524288)
    movt r1, %high(#524288)
    movfs r16,config
    gid
    movfs r18,config
    orr r18,r18,r1
    movts config,r18
    gie
    gid
    movfs r19,config
    orr r19,r19,r1
    movts config,r19
    gie
    imul r0, r0, r17
    ldrb r1,[r3]
    add r0,r0,r1
    mov r1,r2
    add r3,r3,#1
.L2:
    add r2,r1,#-1
    sub r19,r1,#0
    bne .L3
    mov r1, %low(#-917506)
    movt r1, %high(#-917506)
    gid
    movfs r3,config
    eor r3,r3,r16
    and r3,r3,r1
    eor r3,r3,r16
    movts config,r3
    gie
    gid
    movfs r19,config
    eor r19,r19,r16
    and r19,r19,r1
    eor r19,r19,r16
    movts config,r19
    gie
    rts
    .size   _hash, .-_hash
    .ident  "GCC: (Epiphany toolchain 2015.1) 4.8.2 20130729 (prerelease)"

Control will flow from _hash to .L2, never take the branch to .L3, thus never saving config to r16, and proceed to restore ARITHMODE from r16 (that remains unitinialized) before returning.

You might ask, what's the issue? Well, consider that this function might set ARITHMODE to 010. The next time any function attempts integer multiplication, it will assert config bit 19, assuming that it results in an ARITHMODE of 100. In this case, though, that results in an ARITHMODE of 110, which on my E16G301 causes the "FPU/IALU2" pipeline to perform floating-point math.

margnus1 added a commit to margnus1/otp that referenced this issue Jul 16, 2015
Not only does Os not generate notably smaller code, it is also affected
by a compiler bug that is dodged by
O2 (adapteva/epiphany-sdk#53).
margnus1 added a commit to margnus1/otp that referenced this issue Aug 20, 2015
Not only does Os not generate notably smaller code, it is also affected
by a compiler bug that is dodged by
O2 (adapteva/epiphany-sdk#53).
@olajep
Copy link
Contributor

olajep commented Oct 27, 2015

Hi Magnus,

Thank you for reporting. Yes, it is a bug.

Fore future reference, here's a gcc test that also miscompiles because of it:

FAIL: gcc.c-torture/execute/struct-ret-1.c   -Os  execution test

olajep added a commit to adapteva/epiphany-gcc that referenced this issue Nov 4, 2015
Emit gen_save_config instruction right after the prologue so that the
save-register is initialized in all blocks of the function.

Problem described further here:
adapteva/epiphany-sdk#53

Fixes:
FAIL: gcc.c-torture/execute/struct-ret-1.c   -Os  execution test

gcc/
	* config/epiphany/epiphany.c (emit_set_fp_mode): If
	gen_save_config is emitted, insert it right after prologue.

Signed-off-by: Ola Jeppsson <[email protected]>
@olajep olajep changed the title ARITHMODE restore not dominated by save GCC: ARITHMODE restore not dominated by save Apr 18, 2016
olajep added a commit to olajep/epiphany-gcc that referenced this issue Dec 27, 2021
Emit gen_save_config instruction right after the prologue so that the
save-register is initialized in all blocks of the function.

Problem described further here:
adapteva/epiphany-sdk#53

Fixes:
FAIL: gcc.c-torture/execute/struct-ret-1.c   -Os  execution test

gcc/
	* config/epiphany/epiphany.c (emit_set_fp_mode): If
	gen_save_config is emitted, insert it right after prologue.

Signed-off-by: Ola Jeppsson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants