Project

General

Profile

Bug #8358

TestSprintf#test_float test failure

Added by phasis68 (Heesob Park) over 4 years ago. Updated about 3 years ago.

Status:
Closed
Priority:
Normal
Target version:
ruby -v:
ruby 2.1.0dev (2013-05-01) [i386-mingw32]
[ruby-core:<unknown>]

Description

I noticed TestSprintf#test_float
http://ci.rubyinstaller.org/job/ruby-trunk-x86-test-all/1287/console

  1) Failure:
TestSprintf#test_float [C:/Users/Worker/Jenkins/workspace/ruby-trunk-x86-build/test/ruby/test_sprintf.rb:193]:
.
<"0x1p+2"> expected but was
<"0x1p+1">.

This failure is due to r40404.

And Actually, this issue is almost same to bug #8299.
ruby_hdtoa function requires 53-bit precision
but mingw32 compiler is 64-bit precision.

There are 2 possible workarounds.

  1. adding -msse2 -mfpmath=sse flag when compiling.
  2. adding _control87(_PC_53, _MCW_PC) when running.

Related issues

Related to Ruby trunk - Bug #8299: Minor error in float parsingAssigned2013-04-20
Related to Backport21 - Backport #9495: Bacport #8358 - TestSprintf#test_float test failuerClosed2014-02-06
Related to Ruby trunk - Bug #10120: TestSprintf#test_float still an issueClosed2014-08-09

Associated revisions

Revision 44474
Added by nobu (Nobuyoshi Nakada) over 3 years ago

configure.in: use SSE2

  • configure.in: use SSE2 instructions for drop unexpected precisions. [Bug #8358]

Revision 44474
Added by nobu (Nobuyoshi Nakada) over 3 years ago

configure.in: use SSE2

  • configure.in: use SSE2 instructions for drop unexpected precisions. [Bug #8358]

Revision 44474
Added by nobu (Nobuyoshi Nakada) over 3 years ago

configure.in: use SSE2

  • configure.in: use SSE2 instructions for drop unexpected precisions. [Bug #8358]

Revision 44474
Added by nobu (Nobuyoshi Nakada) over 3 years ago

configure.in: use SSE2

  • configure.in: use SSE2 instructions for drop unexpected precisions. [Bug #8358]

Revision 44538
Added by nobu (Nobuyoshi Nakada) over 3 years ago

configure.in: use SSE2

  • configure.in: use SSE2 instructions to drop unexpected precisions on other than mingw. [Bug #8358]

Revision 44538
Added by nobu (Nobuyoshi Nakada) over 3 years ago

configure.in: use SSE2

  • configure.in: use SSE2 instructions to drop unexpected precisions on other than mingw. [Bug #8358]

Revision 44538
Added by nobu (Nobuyoshi Nakada) over 3 years ago

configure.in: use SSE2

  • configure.in: use SSE2 instructions to drop unexpected precisions on other than mingw. [Bug #8358]

Revision 44538
Added by nobu (Nobuyoshi Nakada) over 3 years ago

configure.in: use SSE2

  • configure.in: use SSE2 instructions to drop unexpected precisions on other than mingw. [Bug #8358]

Revision 44890
Added by nobu (Nobuyoshi Nakada) over 3 years ago

configure.in: Properly detect platform for SSE2 instructions.

  • configure.in: add qouting brackets and append wildcard for the rest after target_cpu, to properly detect platform for SSE2 instructions. [Bug #8358]

Revision 44890
Added by nobu (Nobuyoshi Nakada) over 3 years ago

configure.in: Properly detect platform for SSE2 instructions.

  • configure.in: add qouting brackets and append wildcard for the rest after target_cpu, to properly detect platform for SSE2 instructions. [Bug #8358]

Revision 44890
Added by nobu (Nobuyoshi Nakada) over 3 years ago

configure.in: Properly detect platform for SSE2 instructions.

  • configure.in: add qouting brackets and append wildcard for the rest after target_cpu, to properly detect platform for SSE2 instructions. [Bug #8358]

Revision 44890
Added by nobu (Nobuyoshi Nakada) over 3 years ago

configure.in: Properly detect platform for SSE2 instructions.

  • configure.in: add qouting brackets and append wildcard for the rest after target_cpu, to properly detect platform for SSE2 instructions. [Bug #8358]

Revision 45954
Added by shirosaki over 3 years ago

configure.in: enable SSE2 on mingw

  • configure.in: enable SSE2 on mingw. target='i386-pc-mingw32'. [Bug #8358]

Revision 45954
Added by shirosaki over 3 years ago

configure.in: enable SSE2 on mingw

  • configure.in: enable SSE2 on mingw. target='i386-pc-mingw32'. [Bug #8358]

Revision 45954
Added by shirosaki over 3 years ago

configure.in: enable SSE2 on mingw

  • configure.in: enable SSE2 on mingw. target='i386-pc-mingw32'. [Bug #8358]

Revision 45954
Added by shirosaki over 3 years ago

configure.in: enable SSE2 on mingw

  • configure.in: enable SSE2 on mingw. target='i386-pc-mingw32'. [Bug #8358]

Revision 46467
Added by nagachika (Tomoyuki Chikanaga) about 3 years ago

merge revision(s) r45954: [Backport #8358]

* configure.in: enable SSE2 on mingw. target='i386-pc-mingw32'.
   [Bug #8358]

History

#1 [ruby-core:55021] Updated by h.shirosaki (Hiroshi Shirosaki) over 4 years ago

The author describes that it is necessary to specify double-precision (53-bit) rounding precision before invoking strtod or dtoa.
https://github.com/ruby/ruby/blob/trunk/util.c#L526

So that I created a patch to set _PC_53 temporally when calling strtod, dtoa and hdtoa on mingw x86.

It seems that SSE2_MATH is defined only if compiling with -msse2 -mfpmath=sse. In the case it avoids to set _PC_53.

This patch would also fix #8299.

https://gist.github.com/shirosaki/5596940

#2 [ruby-core:55057] Updated by luislavena (Luis Lavena) about 4 years ago

  • Category set to build
  • Status changed from Open to Assigned
  • Assignee set to naruse (Yui NARUSE)
  • Target version set to 2.1.0

naruse: Can we commit the patch?

#3 [ruby-core:55069] Updated by naruse (Yui NARUSE) about 4 years ago

  • Assignee changed from naruse (Yui NARUSE) to nobu (Nobuyoshi Nakada)

As I wrote in #8299, up to nobu.

Our choice will be one of the following:
* add _control87(_PC_53, _MCW_PC) (or inline assembler) to some functions
* add _control87(_PC_53, _MCW_PC) to Init_Numeric
* remove the test

#4 [ruby-core:55848] Updated by phasis68 (Heesob Park) about 4 years ago

Is there any progress about this issue?

This is the only test failure persisted over 2 months.

http://ci.rubyinstaller.org/job/ruby-trunk-x86-test-all/1742/console

#5 [ruby-core:55849] Updated by luislavena (Luis Lavena) about 4 years ago

  • Priority changed from Normal to 5

nobu: ping?

#6 [ruby-core:57547] Updated by vo.x (Vit Ondruch) almost 4 years ago

I observe the same issue on Fedora Rawhide i386.

$ ruby -v
ruby 2.1.0dev (2013-09-22 trunk 43011) [i686-linux]

#7 [ruby-core:58766] Updated by phasis68 (Heesob Park) over 3 years ago

This is a very long standing test failure since 2013-03-23.
If you think this is a intended behavior and don't want to apply any patch, please remove this test case.

#8 [ruby-core:59409] Updated by luislavena (Luis Lavena) over 3 years ago

  • Priority changed from 5 to 7

Hello Nobu, Usa,

This is still happening in 2.1.0 release, and the test is blocking me from releasing RubyInstaller.

We need a response on this, is the test valid or not? can be ignored or the issue needs to be fixed?

Thank you.

#9 Updated by nobu (Nobuyoshi Nakada) over 3 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r44474.
Heesob, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


configure.in: use SSE2

  • configure.in: use SSE2 instructions for drop unexpected precisions. [Bug #8358]

#10 [ruby-core:59472] Updated by vo.x (Vit Ondruch) over 3 years ago

  • Subject changed from TestSprintf#test_float test failuer on mingw32 to TestSprintf#test_float test failuer
  • Status changed from Closed to Assigned
  • Target version changed from 2.1.0 to 2.2.0

Sorry, but this is not Mingw specific issue. I am facing the same issue on Fedora [1]. The test passes on rubyci.org, because there seems to be explicitly specified -msse2 configuration option for some reasons (may be to make this test pass, but that is just speculation), but we are not using this option by default on Fedora i686.

[1] http://kojipkgs.fedoraproject.org//work/tasks/9943/6349943/build.log

#11 [ruby-core:59637] Updated by h.shirosaki (Hiroshi Shirosaki) over 3 years ago

In the case of using SSE2, -mstackrealign flag would be required for Windows.

See #8349

#12 Updated by nobu (Nobuyoshi Nakada) over 3 years ago

  • Status changed from Assigned to Closed

This issue was solved with changeset r44538.
Heesob, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


configure.in: use SSE2

  • configure.in: use SSE2 instructions to drop unexpected precisions on other than mingw. [Bug #8358]

#13 [ruby-core:60576] Updated by vo.x (Vit Ondruch) over 3 years ago

It still does not for for me. It seems that there are two reasons:

1) The [i[4-6]86] if wrongly expanded to i4-686 which does not exist. It should be enclosed in additional brackets.
2) For Fedora, the $target is acutally "i686-redhat-linux-gnu", so there should be wildcard to match the rest of the target string.

The attached patch fixes both issues.

#14 [ruby-core:60579] Updated by nobu (Nobuyoshi Nakada) over 3 years ago

  • Status changed from Assigned to Closed

Applied in changeset r44890.


configure.in: Properly detect platform for SSE2 instructions.

  • configure.in: add qouting brackets and append wildcard for the rest after target_cpu, to properly detect platform for SSE2 instructions. [Bug #8358]

#17 [ruby-core:60599] Updated by naruse (Yui NARUSE) over 3 years ago

  • Status changed from Closed to Assigned

#18 [ruby-core:60606] Updated by nobu (Nobuyoshi Nakada) over 3 years ago

Can't you show its config.log file?

#19 [ruby-core:60617] Updated by akr (Akira Tanaka) over 3 years ago

I committed r44896 to fix the compilation error.

#20 [ruby-core:61493] Updated by zzak (Zachary Scott) over 3 years ago

  • Assignee changed from nobu (Nobuyoshi Nakada) to naruse (Yui NARUSE)

naruse (Yui NARUSE) could you verify that r44896 fixed this on 32bit linux?

#21 [ruby-core:61563] Updated by anatolik (Anatol Pomozov) over 3 years ago

I maintain ruby package in Linux Arch and our users reported an issue with it https://bugs.archlinux.org/task/39470

We compile 32-bit binaries for old machines (PentiumPro+ CPU). The old CPUs do not have SSE2 support, but we've found that our 32-bit kernels contain SSE2 operations.

$ objdump -d /usr/lib/libruby.so | grep movapd | head
2c743: 66 0f 28 c8 movapd %xmm0,%xmm1
2c7e5: 66 0f 28 9b 10 89 f4 movapd -0xb76f0(%ebx),%xmm3
2c800: 66 0f 28 f3 movapd %xmm3,%xmm6
2c808: 66 0f 28 fb movapd %xmm3,%xmm7
32676: 66 0f 28 c8 movapd %xmm0,%xmm1
765d5: 66 0f 28 8b 10 89 f4 movapd -0xb76f0(%ebx),%xmm1

After I reverted the whole sse2 block

-    AS_CASE(["$target"],
-       [*-darwin*], [
-           # doesn't seem necessary on Mac OS X
-       ],
-       [[i[4-6]86*]], [
-           RUBY_TRY_CFLAGS(-msse2 -mfpmath=sse, [
-               RUBY_APPEND_OPTION(XCFLAGS, -msse2 -mfpmath=sse)
-           ])
-            AS_CASE(["$XCFLAGS"],
-                [[*-msse2*]], [
-                    RUBY_TRY_CFLAGS(-mstackrealign, [
-                        RUBY_APPEND_OPTION(XCFLAGS, -mstackrealign)
-                    ])
-                ])
-       ]
-    )

i686 binaries do not contain sse2 operations anymore, x86_64 (64 bit binaries for newer CPU) contain sse2 as expected. Tests "make test" are passed.

Here are the compilation flags uses on Arch:

config.status: creating x86_64-linux-fake.rb
    CC = gcc
    LD = ld
    LDSHARED = gcc -shared
    CFLAGS = -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector --param=ssp-buffer-size=4 -fPIC 
    XCFLAGS = -D_FORTIFY_SOURCE=2 -fstack-protector -fno-strict-overflow -fvisibility=hidden -DRUBY_EXPORT
    CPPFLAGS = -D_FORTIFY_SOURCE=2   -I. -I.ext/include/x86_64-linux -I./include -I.
    DLDFLAGS = -Wl,-soname,libruby.so.2.1  -fstack-protector  
    SOLIBS = -lpthread -lgmp -ldl -lcrypt -lm  

Note to "-march=x86-64" it is for 64-bit builds, on 32-bit builds it is "-march=i686". I believe this flags are enough to tell gcc whether sse2 should be enabled. compiler (at least on Linux Arch) does not need additional sse2 compilation flags.

So my question: why to enable sse2 if the binary is compiled for i686 microarchitecture? "i686" is PentiumPro family that does not support sse2, sse2 is supported only starting from pentium4. See http://gcc.gnu.org/onlinedocs/gcc-4.8.2/gcc/i386-and-x86-64-Options.html#i386-and-x86-64-Options

#22 [ruby-core:61724] Updated by anatolik (Anatol Pomozov) over 3 years ago

We've decided to revert the whole sse2 block from configure.in in official Linux Arch ruby package. https://www.archlinux.org/packages/extra/x86_64/ruby/

That change breaks our users who uses 32-bit packages on old hardware that does not support SSE2.

#23 [ruby-core:61858] Updated by zzak (Zachary Scott) over 3 years ago

  • Subject changed from TestSprintf#test_float test failuer to TestSprintf#test_float test failure
  • Status changed from Assigned to Feedback
  • Assignee changed from naruse (Yui NARUSE) to zzak (Zachary Scott)
  • Priority changed from 7 to Normal

#24 [ruby-core:61859] Updated by luislavena (Luis Lavena) over 3 years ago

@Zachary

This has been waiting review by Nobu/usa in a long time, why is assigned to you? are you going to revert the SSE2 block?

#25 [ruby-core:62026] Updated by hansdegraaff (Hans de Graaff) over 3 years ago

This change is also causing issues for our Gentoo users that still use x86-based systems. See https://bugs.gentoo.org/show_bug.cgi?id=503804 for our downstream bug. We've also dropped the sse2 configure.in block for now to allow compilation on x86 hardware.

#26 [ruby-core:62032] Updated by zzak (Zachary Scott) over 3 years ago

  • Assignee changed from zzak (Zachary Scott) to nobu (Nobuyoshi Nakada)

luis (Luis Lopez) I was using assign as a reminder to check back on this ticket, when I have more time to review Anatol's response.

I think nobu will be able to decide about removing the SSE2 code. Admittedly it sucks that downstream package maintainers have to maintain separate configuration.

#27 [ruby-core:62095] Updated by h.shirosaki (Hiroshi Shirosaki) over 3 years ago

I found SSE2 is not enabled on mingw because of target='i386-pc-mingw32'.
Here is a patch.

diff --git a/configure.in b/configure.in
index 17ed3ed..cc684b5 100644
--- a/configure.in
+++ b/configure.in
@@ -860,7 +860,7 @@ if test "$GCC" = yes; then
        [*-darwin*], [
            # doesn't seem necessary on Mac OS X
        ],
-       [[i[4-6]86*]], [
+       [[i[4-6]86*|i386*mingw*]], [
            RUBY_TRY_CFLAGS(-msse2 -mfpmath=sse, [
                RUBY_APPEND_OPTION(XCFLAGS, -msse2 -mfpmath=sse)
            ])

#28 [ruby-core:62529] Updated by luislavena (Luis Lavena) over 3 years ago

  • Status changed from Feedback to Assigned

Hello nobu,

Ping on this? this is still an issue on trunk and ruby_2_1 branch:

TestSprintf#test_float [C:/Users/Luis/Code/ruby/ruby/test/ruby/test_sprintf.rb:198]:
.
<"0x1p+2"> expected but was
<"0x1p+1">.

(ruby 2.2.0dev (2014-05-11 trunk 45921) [i386-mingw32])

If you don't object, going to apply Shirosaki's patch and request backport to 2.1.

Thank you.

#30 [ruby-core:62541] Updated by nobu (Nobuyoshi Nakada) over 3 years ago

Luis Lavena wrote:

If you don't object, going to apply Shirosaki's patch and request backport to 2.1.

Go ahead.

#31 [ruby-core:62542] Updated by nobu (Nobuyoshi Nakada) over 3 years ago

  • Description updated (diff)

#32 [ruby-core:62547] Updated by zzak (Zachary Scott) over 3 years ago

  • Assignee changed from nobu (Nobuyoshi Nakada) to luislavena (Luis Lavena)

#33 Updated by Anonymous over 3 years ago

  • Status changed from Assigned to Closed

Applied in changeset r45954.


configure.in: enable SSE2 on mingw

  • configure.in: enable SSE2 on mingw. target='i386-pc-mingw32'. [Bug #8358]

#34 [ruby-core:63201] Updated by nobu (Nobuyoshi Nakada) about 3 years ago

  • Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN to 2.0.0: REQUIRED, 2.1: REQUIRED

#35 [ruby-core:63250] Updated by nagachika (Tomoyuki Chikanaga) about 3 years ago

  • Related to Backport #9495: Bacport #8358 - TestSprintf#test_float test failuer added

#36 [ruby-core:63252] Updated by nagachika (Tomoyuki Chikanaga) about 3 years ago

  • Backport changed from 2.0.0: REQUIRED, 2.1: REQUIRED to 2.0.0: REQUIRED, 2.1: DONE

r44474, r44538, r44539, r44890 and r44896 were already backported into ruby_2_1 at r44993.
And r45954 was backported at r46467.

#37 [ruby-core:63292] Updated by usa (Usaku NAKAMURA) about 3 years ago

  • Backport changed from 2.0.0: REQUIRED, 2.1: DONE to 2.0.0: DONE, 2.1: DONE

r44474, r44538, r44539, r44890, r44896 and r45954 were backported into ruby_2_0_0 at r46514.

#38 [ruby-core:64285] Updated by vo.x (Vit Ondruch) about 3 years ago

  • Related to Bug #10120: TestSprintf#test_float still an issue added

Also available in: Atom PDF