XS::APItest::test_coplabel fails on HP-UX with aCC +O2 optimization enabled #11748
Comments
From @icerider70Created by philip.monsen@pobox.comUnder +O1 compiler optimization, the test passes fine: $ ./perl -I./lib ext/XS-APItest/t/coplabel.t But when +O2 is used, not so much: $ ./perl -I./lib ext/XS-APItest/t/coplabel.t I've filed this as a bug because the default -Doptimize value I believe the issue is partly due to reordering of instructions. XS_EUPXS(XS_XS__APItest_test_coplabel) Here's a snippet from a gdb debugging session under +O1: 2649 Perl_cop_store_label(aTHX_ cop, "foo", 3, 0); Note the contrast when +O2 is in force: 2650 label = Perl_cop_fetch_label(aTHX_ cop, &len, &utf8); Due to the opaqueness imposed by the +O2 optimizations, it's hard Reordering the tests within this function, and changing strcmp() Patch forthcoming. Perl Info |
From zefram@fysh.orgPhilip Monsen wrote:
Presumably the same optimisation bug will strike in other places that -zefram |
|
The RT System itself - Status changed from 'new' to 'open' |
From @icerider70On Tue, Nov 15, 2011 at 10:43 AM, Zefram <zefram@fysh.org> wrote:
I thought about that, but it seemed like overkill considering that all Someone more familiar with the core code being tested could probably revise --Phil |
From @icerider70The attached patch corrects the issue. To illustrate the difference 2650 label = Perl_cop_fetch_label(aTHX_ cop, &len, &utf8); Note how the optimization no longer induces a bogus failure and early exit. |
From @icerider700001-Make-test_coplabel-more-optimization-safe.patchFrom 009dcc517b2bf746c1a65640a1c89d80ca4589f2 Mon Sep 17 00:00:00 2001
From: Philip Monsen <philip.monsen@pobox.com>
Date: Tue, 15 Nov 2011 09:51:52 -0600
Subject: [PATCH] Make test_coplabel() more optimization safe
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.7.7.3"
This is a multi-part message in MIME format.
--------------1.7.7.3
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit
---
ext/XS-APItest/APItest.xs | 15 ++++++++-------
1 files changed, 8 insertions(+), 7 deletions(-)
--------------1.7.7.3
Content-Type: text/x-patch; name="0001-Make-test_coplabel-more-optimization-safe.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0001-Make-test_coplabel-more-optimization-safe.patch"
diff --git a/ext/XS-APItest/APItest.xs b/ext/XS-APItest/APItest.xs
index 646d821..68ce3f5 100644
--- a/ext/XS-APItest/APItest.xs
+++ b/ext/XS-APItest/APItest.xs
@@ -2648,16 +2648,17 @@ test_coplabel()
cop = &PL_compiling;
Perl_cop_store_label(aTHX_ cop, "foo", 3, 0);
label = Perl_cop_fetch_label(aTHX_ cop, &len, &utf8);
- if (strcmp(label,"foo")) croak("fail # cop_fetch_label label");
- if (len != 3) croak("fail # cop_fetch_label len");
- if (utf8) croak("fail # cop_fetch_label utf8");
+ if (len != 3) croak("fail # test1 cop_fetch_label len");
+ if (memcmp(label,"foo",len))
+ croak("fail # test1 cop_fetch_label label");
+ if (utf8) croak("fail # test1 cop_fetch_label utf8");
/* SMALL GERMAN UMLAUT A */
Perl_cop_store_label(aTHX_ cop, "fo��", 4, SVf_UTF8);
label = Perl_cop_fetch_label(aTHX_ cop, &len, &utf8);
- if (strcmp(label,"fo��")) croak("fail # cop_fetch_label label");
- if (len != 4) croak("fail # cop_fetch_label len");
- if (!utf8) croak("fail # cop_fetch_label utf8");
-
+ if (len != 4) croak("fail # test2 cop_fetch_label len");
+ if (memcmp(label,"fo��",len))
+ croak("fail # test2 cop_fetch_label label");
+ if (!utf8) croak("fail # test2 cop_fetch_label utf8");
HV *
example_cophh_2hv()
--------------1.7.7.3--
|
From zefram@fysh.orgPhilip Monsen wrote:
The patch has a small fault, that the new code with memcmp() isn't However, looking at the change, I am even less happy about patching it We need to turn off the broken optimisation. If that means dropping -zefram |
From @icerider70On Wed, Nov 16, 2011 at 7:41 AM, Zefram <zefram@fysh.org> wrote:
Fair enough. I'll try using +O2 as a base and pulling specific --Phil |
From @icerider70On Wed, Nov 16, 2011 at 8:19 AM, Philip Monsen <philip.monsen@pobox.com>wrote:
I added +Oinfo to see what optimizations are actually occurring. Looks "APItest.c", line 2651, procedure XS_XS__APItest_test_coplabel: info "APItest.c", line 2657, procedure XS_XS__APItest_test_coplabel: info Default for +O2 is '+inline_level 2' and for +O1 is '+inline_level 1'. --Phil |
From @icerider70On Wed, Nov 16, 2011 at 9:30 AM, Philip Monsen <philip.monsen@pobox.com>wrote:
This wasn't quite the right tweak, but the +Oinfo report was helpful. +O2 +Onoinline However, using '+O2 +Onolibcalls=strcmp' results in this test passing. 2650 label = Perl_cop_fetch_label(aTHX_ cop, &len, &utf8); I'm going to rebuild once more with '+O2 +Onolimit +Olibcalls=strcmp', and --Phil |
From @icerider70On Wed, Nov 16, 2011 at 11:08 AM, Philip Monsen <philip.monsen@pobox.com>wrote:
Promised patch (to hints/hpux.sh) against blead attached. This one also I added some additional relaxations to the optimize string, as the +O2 +Onolimit +Onoprocelim +Ostore_ordering +Onolibcalls=strcmp also seems reasonable to me given the initial observed behavior, and avoids With this patch applied, all tests in the test suite pass when the --Phil |
From @icerider700001-Relax-level-2-optimizations-to-fix-test-failures-on-.patchFrom f4ccbf7554962be4d37838734f7d6c842e12691b Mon Sep 17 00:00:00 2001
From: Philip Monsen <philip.monsen@pobox.com>
Date: Thu, 17 Nov 2011 11:22:17 -0600
Subject: [PATCH] Relax level 2 optimizations to fix test failures on HP-UX
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------1.7.7.3"
This is a multi-part message in MIME format.
--------------1.7.7.3
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit
---
hints/hpux.sh | 17 +++++++++++++++--
1 files changed, 15 insertions(+), 2 deletions(-)
--------------1.7.7.3
Content-Type: text/x-patch; name="0001-Relax-level-2-optimizations-to-fix-test-failures-on-.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0001-Relax-level-2-optimizations-to-fix-test-failures-on-.patch"
diff --git a/hints/hpux.sh b/hints/hpux.sh
index be6c1fd..64de1bd 100644
--- a/hints/hpux.sh
+++ b/hints/hpux.sh
@@ -157,7 +157,7 @@ case `$cc -v 2>&1`"" in
done
[ -z "$cc_found" ] && cc_found=`which cc`
what $cc_found >&4
- ccversion=`what $cc_found | awk '/Compiler/{print $2}/Itanium/{print $6,$7}/for Integrity/{print $6}'`
+ ccversion=`what $cc_found | awk '/Compiler/{print $2}/Itanium/{print $6,$7}/for Integrity/{print $6,$7}'`
case "$ccflags" in
"-Ae "*) ;;
*) ccflags="-Ae $cc_cppflags"
@@ -414,7 +414,7 @@ case "$ccisgcc" in
fi
;;
- *) # HP's compiler cannot combine -g and -O
+ *)
case "$optimize" in
"") optimize="+O2 +Onolimit" ;;
*O[3456789]*) optimize=`echo "$optimize" | sed -e 's/O[3-9]/O2/'` ;;
@@ -436,6 +436,19 @@ case "$ccisgcc" in
# maint (5.8.8+) and blead (5.9.3+)
# -O1/+O1 passed all tests (m)'05 [ 10 Jan 2005 ]
optimize="$opt" ;;
+ B3910B*A.06.15)
+ # > cc --version
+ # cc: HP C/aC++ B3910B A.06.15 [May 16 2007]
+ # Has optimizing problems with +O2 for blead (5.15.4),
+ # see https://rt.perl.org:443/rt3/Ticket/Display.html?id=103668.
+ #
+ # +O2 +Onolimit +Onoprocelim +Ostore_ordering \
+ # +Onolibcalls=strcmp
+ # passes all tests (with/without -DDEBUGGING) [Nov 17 2011]
+ case "$optimize" in
+ *O2*) optimize="$optimize +Onoprocelim +Ostore_ordering +Onolibcalls=strcmp" ;;
+ esac
+ ;;
*) doop_cflags="optimize=\"$opt\""
op_cflags="optimize=\"$opt\"" ;;
esac
--------------1.7.7.3--
|
From zefram@fysh.orgPhilip Monsen wrote:
Thanks, applied as 2ba3ed0. -zefram |
|
@cpansprout - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#103668 (status was 'resolved')
Searchable as RT103668$
The text was updated successfully, but these errors were encountered: