Bug 69291 - [6 Regression] wrong code at -O1 for ruby-2.3.0/regcomp.c:985:compile_length_quantifier_node()
Summary: [6 Regression] wrong code at -O1 for ruby-2.3.0/regcomp.c:985:compile_length_...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 6.0
: P1 normal
Target Milestone: 6.0
Assignee: Richard Biener
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2016-01-15 05:38 UTC by Takeshi Nishimatsu
Modified: 2016-08-25 05:16 UTC (History)
2 users (show)

See Also:
Host:
Target: i586-linux
Build:
Known to work:
Known to fail:
Last reconfirmed: 2016-02-10 00:00:00


Attachments
preprocessed source (59.96 KB, text/plain)
2016-02-10 12:05 UTC, Richard Biener
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Takeshi Nishimatsu 2016-01-15 05:38:59 UTC
gcc-6 gives wrong code at -O1 for ruby-2.3.0/regcomp.c:985:compile_length_quantifier_node()
and cannot build the ruby.  Even the -O1 optimization option fails.
Adding __attribute__((optimize("O0"))) before the function can evade this issue.
I confirmed this issue on CentOS 6.7 (x86_64-pc-linux-gnu) and
Mac OS X 10.11.2 (x86_64-apple-darwin15.0.0).

However, I found strange behavior. If I built the ruby in /tmp on CentOS 6.7,
this issue does not stop to `make encdb.h`. But anyway, it stops to `make rdoc`
with Segmentation fault. Adding __attribute__((optimize("O0"))) can eliminate
the Segmentation fault, too.

Please note that there are two compile_length_quantifier_node() in
ruby-2.3.0/regcomp.c. gcc-6 fails to optimize second one at the line 985.

This may be a regression, because gcc-4.9.2 could build ruby-2.3.0.
clang of Mac OS X (Apple LLVM version 7.0.2 (clang-700.1.81)) could
build ruby-2.3.0, too.

================================
$ wget https://cache.ruby-lang.org/pub/ruby/2.3/ruby-2.3.0.tar.xz
$ shasum ruby-2.3.0.tar.xz | grep 96e620e38af351c8da63e40cfe217ec79f912ba1
$ tar xf ruby-2.3.0.tar.xz
$ mkdir  ruby-2.3.0/build_on_MacOSX
$ cd     ruby-2.3.0/build_on_MacOSX
$ /usr/local/bin/gcc --version
gcc (GCC) 6.0.0 20151115 (experimental)
$ ../configure CC=/usr/local/bin/gcc
$ make -j4
    :
generating encdb.h
../template/encdb.h.tmpl:63:in `block (4 levels) in <main>': x_emoji.h:8: ENC_REPLICATE: UTF-8 is not defined yet. (replica UTF8-DoCoMo) (ArgumentError)
    :
$ sed -e '985 i\
> __attribute__((optimize("O0")))' ../regcomp.c > regcomp.c
$ diff -u ../regcomp.c .
--- ../regcomp.c        2015-12-09 16:30:44.000000000 +0900
+++ ./regcomp.c 2016-01-14 08:21:52.798816125 +0900
@@ -982,6 +982,7 @@
 #else /* USE_COMBINATION_EXPLOSION_CHECK */
 static int
+__attribute__((optimize("O0")))
 compile_length_quantifier_node(QtfrNode* qn, regex_t* reg)
 {
   int len, mod_tlen;
$ make V=1 encdb.h
    :
./miniruby -I../lib -I. -I.ext/common  ../tool/generic_erb.rb -c -o encdb.h ../template/encdb.h.tmpl ../enc enc
encdb.h updated
$ make -j4
$ make check
    :
# Running tests:

Finished tests in 453.556568s, 34.7388 tests/s, 4931.0101 assertions/s.
15756 tests, 2236492 assertions, 0 failures, 0 errors, 53 skips

ruby -v: ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-darwin15]
check succeeded
$
================================

Tank you for your constant efforts on GCC,
Takeshi
Comment 1 Markus Trippelsdorf 2016-01-15 05:50:50 UTC
I think this bug is invalid, because ruby invokes undefined behavior, see:

https://bugs.ruby-lang.org/issues/11831
Comment 2 Andrew Pinski 2016-01-15 07:51:16 UTC
.
Comment 3 Markus Trippelsdorf 2016-01-15 13:18:32 UTC
It turned out that this issue is unrelated to the unaligned word accesses.
But fortunately it is already fix on today's trunk.
Comment 4 Richard Biener 2016-02-10 10:37:55 UTC
Issue still happens on i586 and fails the ruby build with

[  149s] gcc -fomit-frame-pointer -fmessage-length=0 -grecord-gcc-switches -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables -fasynchronous-unwind-tables -Wno-error=misleading-indentation -Wno-error=narrowing -Wno-error=nonnull -Wno-error=unused-const-variable -g -fno-strict-aliasing -O -U_FORTIFY_SOURCE -fPIC  -L. -fstack-protector -rdynamic -Wl,-export-dynamic -fstack-protector  main.o dmydln.o miniinit.o miniprelude.o array.o bignum.o class.o compar.o complex.o dir.o dln_find.o encoding.o enum.o enumerator.o error.o eval.o load.o proc.o file.o gc.o hash.o inits.o io.o marshal.o math.o node.o numeric.o object.o pack.o parse.o process.o random.o range.o rational.o re.o regcomp.o regenc.o regerror.o regexec.o regparse.o regsyntax.o ruby.o safe.o signal.o sprintf.o st.o strftime.o string.o struct.o time.o transcode.o util.o variable.o version.o compile.o debug.o iseq.o vm.o vm_dump.o vm_backtrace.o vm_trace.o thread.o cont.o ascii.o us_ascii.o unicode.o utf_8.o newline.o strlcpy.o strlcat.o setproctitle.o addr2line.o  dmyext.o   -lpthread -ldl -lcrypt -lm   -o miniruby
[  149s] ./miniruby -I./lib -I. -I.ext/common  ./tool/mkconfig.rb -timestamp=.rbconfig.time \
[  149s]        -install_name=ruby.ruby2.1 \
[  149s]        -so_name=ruby2.1 rbconfig.rb
[  149s] ./tool/mkconfig.rb:121:in `gsub': undefined bytecode (bug): /\$(?:\$|\{?(\w+)\}?)/ (RegexpError)

the regcomp.c workaround, -O0 for compile_length_quantifier_node still works,
in fact "no-tree-dominator-opts" is enough to fix it.

Poking some more.
Comment 5 Richard Biener 2016-02-10 11:09:36 UTC
Ok, not inlining compile_length_quantifier_node fixes it as well.  It's inlined in
compile_length_tree.

-fdisable-tree-uncprop1 fixes it - that hints more at a target specific RTL issue
or an RTL expansion issue.

-fno-tree-coalesce-vars also fixes it (disabling TER doesn't).
Comment 6 Richard Biener 2016-02-10 12:05:07 UTC
Created attachment 37651 [details]
preprocessed source

Build with -m32 -mtune=generic -march=i586 -O [-fPIC]

-fdisable-rtl-ce2 (if-conversion after combine) fixes it.

IF-THEN-ELSE-JOIN block found, pass 1, test 63, then 64, else 65, join 128
scanning new insn with uid = 842.
scanning new insn with uid = 843.
scanning new insn with uid = 844.
scanning new insn with uid = 845.
scanning new insn with uid = 846.
scanning new insn with uid = 847.
deleting insn with uid = 363.
deleting block 65
Removing jump 354.
deleting insn with uid = 354.
deleting insn with uid = 357.
deleting block 64
Conversion succeeded on pass 1.

IF-CASE-2 found, start 88, else 90
verify found no changes in insn with uid = 471.
deleting block 90
Conversion succeeded on pass 1.

Disabling the IF-THEN-ELSE-JOIN transform above fixes the issue.
Comment 7 Richard Biener 2016-02-10 12:20:07 UTC
It combines

(insn 353 352 354 63 (set (reg:CCZ 17 flags)
        (compare:CCZ (mem/f:SI (plus:SI (reg/v/f:SI 225 [ node ])
                    (const_int 32 [0x20])) [10 MEM[(struct QtfrNode *)node_14(D)].next_head_exact+0 S4 A32])
            (const_int 0 [0]))) regcomp.c:1011 3 {*cmpsi_ccno_1}
     (expr_list:REG_DEAD (reg/v/f:SI 225 [ node ])
        (nil)))
(jump_insn 354 353 355 63 (set (pc)
        (if_then_else (eq (reg:CCZ 17 flags)
                (const_int 0 [0]))
            (label_ref 360)
            (pc))) regcomp.c:1011 635 {*jcc_1}
     (expr_list:REG_DEAD (reg:CCZ 17 flags)
        (int_list:REG_BR_PROB 1500 (nil)))

else:
(insn 357 356 360 64 (set (reg/v:SI 224 [ <retval> ])
        (plus:SI (plus:SI (reg/v:SI 160 [ mod_tlen ])
                (reg/v:SI 224 [ <retval> ]))
            (const_int 11 [0xb]))) regcomp.c:1012 214 {*leasi}
     (expr_list:REG_DEAD (reg/v:SI 160 [ mod_tlen ])
        (nil)))

then:
(insn 363 362 366 65 (set (reg/v:SI 224 [ <retval> ])
        (plus:SI (plus:SI (reg/v:SI 160 [ mod_tlen ])
                (reg/v:SI 224 [ <retval> ]))
            (const_int 10 [0xa]))) regcomp.c:1014 214 {*leasi}
     (expr_list:REG_DEAD (reg/v:SI 160 [ mod_tlen ])
        (nil)))

into (wtf!):

(insn 353 352 842 63 (set (reg:CCZ 17 flags)
        (compare:CCZ (mem/f:SI (plus:SI (reg/v/f:SI 225 [ node ])
                    (const_int 32 [0x20])) [10 MEM[(struct QtfrNode *)node_14(D)].next_head_exact+0 S4 A32])
            (const_int 0 [0]))) regcomp.c:1011 3 {*cmpsi_ccno_1}
     (expr_list:REG_DEAD (reg/v/f:SI 225 [ node ])
        (nil)))
(insn 842 353 843 63 (set (reg:CCZ 17 flags)
        (compare:CCZ (mem/f:SI (plus:SI (reg/v/f:SI 225 [ node ])
                    (const_int 32 [0x20])) [10 MEM[(struct QtfrNode *)node_14(D)].next_head_exact+0 S4 A32])
            (const_int 0 [0]))) regcomp.c:1012 3 {*cmpsi_ccno_1}
     (nil))
(insn 843 842 844 63 (set (reg:QI 331)
        (ne:QI (reg:CCZ 17 flags)
            (const_int 0 [0]))) regcomp.c:1012 631 {*setcc_qi}
     (nil))
(insn 844 843 845 63 (set (reg/v:SI 224 [ <retval> ])
        (zero_extend:SI (reg:QI 331))) regcomp.c:1012 136 {*zero_extendqisi2}
     (nil))
(insn 845 844 846 63 (parallel [
            (set (reg:SI 332)
                (plus:SI (reg/v:SI 160 [ mod_tlen ])
                    (reg/v:SI 224 [ <retval> ])))
            (clobber (reg:CC 17 flags))
        ]) regcomp.c:1012 218 {*addsi_1}
     (nil))
(insn 846 845 847 63 (parallel [
            (set (reg/v:SI 224 [ <retval> ])
                (plus:SI (reg/v:SI 224 [ <retval> ])
                    (reg:SI 332)))
            (clobber (reg:CC 17 flags))
        ]) regcomp.c:1012 218 {*addsi_1}
     (nil))
(insn 847 846 366 63 (parallel [
            (set (reg/v:SI 224 [ <retval> ])
                (plus:SI (reg/v:SI 224 [ <retval> ])
                    (const_int 10 [0xa])))
            (clobber (reg:CC 17 flags))
        ]) regcomp.c:1012 218 {*addsi_1}
     (nil))

where it overwrites 224 with setcc even thouht 224 is needed to compute the
original expression.
Comment 8 Richard Biener 2016-02-10 12:32:59 UTC
#0  noce_emit_store_flag (if_info=0x7fffffffd7d0, x=0x7ffff5ade840, 
    reversep=1, normalize=0) at /space/rguenther/src/svn/trunk3/gcc/ifcvt.c:839
#1  0x00000000017adfa3 in noce_try_store_flag_constants (
    if_info=0x7fffffffd7d0) at /space/rguenther/src/svn/trunk3/gcc/ifcvt.c:1390
#2  0x00000000017b3230 in noce_process_if_block (if_info=0x7fffffffd7d0)
    at /space/rguenther/src/svn/trunk3/gcc/ifcvt.c:3503

and we try to detect this with

      /* If we have x := test ? x + 3 : x + 4 then move the original
         x out of the way while we store flags.  */
      if (common && rtx_equal_p (common, if_info->x))
        {
          common = gen_reg_rtx (mode);
          noce_emit_move_insn (common, if_info->x);
        }

but here

(gdb) p debug_rtx (if_info->x)
(reg/v:SI 224 [ <retval> ])
(gdb) p debug_rtx (common)
(plus:SI (reg/v:SI 160 [ mod_tlen ])
    (reg/v:SI 224 [ <retval> ]))

I suppose it should use reg_mentioned_p instead.  But even then the generated
code is broken.  Seems it doesn't expect a plus as common at all.  It then
copies the reg and does

(insn 842 353 843 63 (set (reg:SI 331)
        (reg/v:SI 224 [ <retval> ])) regcomp.c:1012 86 {*movsi_internal}
     (nil))
(insn 843 842 844 63 (set (reg:CCZ 17 flags)
        (compare:CCZ (mem/f:SI (plus:SI (reg/v/f:SI 225 [ node ])
                    (const_int 32 [0x20])) [10 MEM[(struct QtfrNode *)node_14(D)].next_head_exact+0 S4 A32])
            (const_int 0 [0]))) regcomp.c:1012 3 {*cmpsi_ccno_1}
     (nil))
(insn 844 843 845 63 (set (reg:QI 332)
        (ne:QI (reg:CCZ 17 flags)
            (const_int 0 [0]))) regcomp.c:1012 631 {*setcc_qi}
     (nil))
(insn 845 844 846 63 (set (reg/v:SI 224 [ <retval> ])
        (zero_extend:SI (reg:QI 332))) regcomp.c:1012 136 {*zero_extendqisi2}
     (nil))
(insn 846 845 847 63 (parallel [
            (set (reg/v:SI 224 [ <retval> ])
                (plus:SI (reg/v:SI 224 [ <retval> ])
                    (reg:SI 331)))
            (clobber (reg:CC 17 flags))
        ]) regcomp.c:1012 218 {*addsi_1}
     (nil))
(insn 847 846 366 63 (parallel [
            (set (reg/v:SI 224 [ <retval> ])
                (plus:SI (reg/v:SI 224 [ <retval> ])
                    (const_int 10 [0xa])))
            (clobber (reg:CC 17 flags))
        ]) regcomp.c:1012 218 {*addsi_1}
     (nil))

and so loses the reg:SI 160 addend.
Comment 9 Richard Biener 2016-02-10 12:38:22 UTC
Ah, of course

      /* If we have x := test ? x + 3 : x + 4 then move the original
         x out of the way while we store flags.  */
      if (common && reg_mentioned_p (if_info->x, common))
        {
          common = gen_reg_rtx (mode);
          noce_emit_move_insn (common, if_info->x);
        }

can't work - we'll overwrite 'common' here.

      /* If we have x := test ? x + 3 : x + 4 then move the original
         x out of the way while we store flags.  */
      if (common && reg_mentioned_p (if_info->x, common))
        {
          rtx tem = gen_reg_rtx (mode);
          noce_emit_move_insn (tem, common);
          common = tem;
        }

does.  Of course that insn might not be recognizable.  The above fixes the ruby failure for me.
Comment 10 Richard Biener 2016-02-10 12:41:57 UTC
Testing

Index: gcc/ifcvt.c
===================================================================
--- gcc/ifcvt.c (revision 233262)
+++ gcc/ifcvt.c (working copy)
@@ -1381,10 +1381,11 @@ noce_try_store_flag_constants (struct no
 
       /* If we have x := test ? x + 3 : x + 4 then move the original
         x out of the way while we store flags.  */
-      if (common && rtx_equal_p (common, if_info->x))
+      if (common && reg_mentioned_p (if_info->x, common))
        {
-         common = gen_reg_rtx (mode);
-         noce_emit_move_insn (common, if_info->x);
+         rtx tem = gen_reg_rtx (mode);
+         noce_emit_move_insn (tem, common);
+         common = tem;
        }
 
       target = noce_emit_store_flag (if_info, if_info->x, reversep, normalize);
Comment 11 Richard Biener 2016-02-10 12:45:43 UTC
Caused by r226869.
Comment 12 Richard Biener 2016-02-11 08:12:13 UTC
Fixed.
Comment 13 Richard Biener 2016-02-11 08:12:25 UTC
Author: rguenth
Date: Thu Feb 11 08:11:52 2016
New Revision: 233316

URL: https://gcc.gnu.org/viewcvs?rev=233316&root=gcc&view=rev
Log:
2016-02-11  Richard Biener  <rguenther@suse.de>

	PR rtl-optimization/69291
	* ifcvt.c (noce_try_store_flag_constants): Do not allow
	subexpressions affected by changing the result.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ifcvt.c
Comment 14 Richard Biener 2016-02-16 10:53:40 UTC
Author: rguenth
Date: Tue Feb 16 10:53:08 2016
New Revision: 233448

URL: https://gcc.gnu.org/viewcvs?rev=233448&root=gcc&view=rev
Log:
2016-02-16  Richard Biener  <rguenther@suse.de>

	PR rtl-optimization/69291
	* ifcvt.c (noce_try_store_flag_constants): Re-instantiate
	noce_operand_ok check.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ifcvt.c