diff mbox

x86-64: Align L(SP_RANGE)/L(SP_INF_0) to 8 bytes [BZ #21955]

Message ID CAMe9rOpRW5_hoN3p0XOhtvKs9UiVsLgKiba-R7yDidzyTCyL3g@mail.gmail.com
State New, archived
Headers show

Commit Message

H.J. Lu Aug. 15, 2017, 8:56 p.m. UTC
On Tue, Aug 15, 2017 at 1:28 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Tue, 15 Aug 2017, H.J. Lu wrote:
>
>> On Tue, Aug 15, 2017 at 1:02 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>> > On Tue, 15 Aug 2017, H.J. Lu wrote:
>> >
>> >> >     [BZ #21955]
>> >> >     * sysdeps/x86_64/fpu/e_expf.S (L(SP_INF_0)): Place it in
>> >> >     .rodata.cst4 section.
>> >>
>> >> L(SP_RANGE) has the same issue.  This updated patch fixes both.
>> >
>> > It's a lot more than just expf.  There are various other x86_64 and x86
>> > libm files that could use .rodata.cstN but don't.  (Obviously this only
>> > works for invididual objects where the code doesn't use offsets from one
>> > object to another, not when an array of two or more objects is being used
>> > unless you choose the section appropriately to preserve the array as
>> > such.)
>>
>> Aren't .rodata.cstN optimization?  In case of e_expf.S, it is a correctness
>> issue.
>
> They are generally optimization.
>
> Could you explain this correctness issue in more detail?  SP_INF_0 appears
> to be an array of two 4-byte values.  Because it's used as an array, the
> two values need to stay adjacent.  That is, I'd expect it to need, for
> correctness, to be in .rodata.cst8, as it is at present, and *not*
> .rodata.cst4 (if in .rodata.cst4, it might get split up as those values
> get unified with other values in that section).
>

After a closer look, the bug is

         .section .rodata.cst8,"aM",@progbits,8
...
        .p2align 2  <<<<<< 4 byte aligned.

Here is the new patch.  OK for master?

Comments

Joseph Myers Aug. 15, 2017, 9:03 p.m. UTC | #1
On Tue, 15 Aug 2017, H.J. Lu wrote:

> After a closer look, the bug is
> 
>          .section .rodata.cst8,"aM",@progbits,8
> ...
>         .p2align 2  <<<<<< 4 byte aligned.
> 
> Here is the new patch.  OK for master?

This is OK.
diff mbox

Patch

From 39245565fc0523eece29721c4590639ccebb6145 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 15 Aug 2017 10:34:22 -0700
Subject: [PATCH] x86-64: Align L(SP_RANGE)/L(SP_INF_0) to 8 bytes [BZ #21955]

sysdeps/x86_64/fpu/e_expf.S has

        lea     L(SP_RANGE)(%rip), %rdx /* load over/underflow bound */
        cmpl    (%rdx,%rax,4), %ecx     /* |x|<under/overflow bound ? */
...
        /* Here if |x| is Inf */
        lea     L(SP_INF_0)(%rip), %rdx /* depending on sign of x: */
        movss   (%rdx,%rax,4), %xmm0    /* return zero or Inf */
        ret
...
         .section .rodata.cst8,"aM",@progbits,8
...
        .p2align 2
L(SP_RANGE): /* single precision overflow/underflow bounds */
        .long   0x42b17217      /* if x>this bound, then result overflows */
        .long   0x42cff1b4      /* if x<this bound, then result underflows */
        .type L(SP_RANGE), @object
        ASM_SIZE_DIRECTIVE(L(SP_RANGE))

        .p2align 2
L(SP_INF_0):
        .long   0x7f800000      /* single precision Inf */
        .long   0               /* single precision zero */
        .type L(SP_INF_0), @object
        ASM_SIZE_DIRECTIVE(L(SP_INF_0))

Since L(SP_RANGE) and L(SP_INF_0) are in .rodata.cst8 section, they
must be aligned to 8 bytes.

	[BZ #21955]
	* sysdeps/x86_64/fpu/e_expf.S (L(SP_RANGE)): Aligned to 8 bytes.
	(L(SP_INF_0)): Likewise.
---
 sysdeps/x86_64/fpu/e_expf.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sysdeps/x86_64/fpu/e_expf.S b/sysdeps/x86_64/fpu/e_expf.S
index 4fd2bb1fb5..c3bf312c44 100644
--- a/sysdeps/x86_64/fpu/e_expf.S
+++ b/sysdeps/x86_64/fpu/e_expf.S
@@ -297,14 +297,14 @@  L(DP_P0): /* double precision polynomial coefficient P0 */
 	.type L(DP_P0), @object
 	ASM_SIZE_DIRECTIVE(L(DP_P0))
 
-	.p2align 2
+	.p2align 3
 L(SP_RANGE): /* single precision overflow/underflow bounds */
 	.long	0x42b17217	/* if x>this bound, then result overflows */
 	.long	0x42cff1b4	/* if x<this bound, then result underflows */
 	.type L(SP_RANGE), @object
 	ASM_SIZE_DIRECTIVE(L(SP_RANGE))
 
-	.p2align 2
+	.p2align 3
 L(SP_INF_0):
 	.long	0x7f800000	/* single precision Inf */
 	.long	0		/* single precision zero */
-- 
2.13.5