Browse files

[imcc] fix -O2 used_once: keep side-effecting ops #1036

allow used_once elimination only for purely functional ops
without side-effects.
Fixes t/compilers/imcc/reg/alloc.t test 2 alligator 2 - r9629
      t/compilers/imcc/reg/spill_old.t test 1 bug \#32996
and some more.

set ITPUREFUNC in the parser, as this is the best place to find them:
logical un_op and null; artithmetic bin_op and assign_op
  • Loading branch information...
1 parent 0ff8b80 commit 74055e5f252bc5863cd8de2701705bbcd327209f @rurban rurban committed Feb 26, 2014
View
14 compilers/imcc/imcc.y
@@ -1975,7 +1975,8 @@ labeled_inst:
imcc->keyvec, 1);
mem_sys_free($1);
}
- | PNULL var { $$ = MK_I(imcc, imcc->cur_unit, "null", 1, $2); }
+ | PNULL var { $$ = MK_I(imcc, imcc->cur_unit, "null", 1, $2);
+ $$->type = ITPUREFUNC; }
| sub_call { $$ = 0; imcc->cur_call = NULL; }
| pcc_sub_call { $$ = 0; }
| pcc_ret
@@ -1994,9 +1995,11 @@ assignment:
target '=' var
{ $$ = MK_I(imcc, imcc->cur_unit, "set", 2, $1, $3); }
| target '=' un_op var
- { $$ = MK_I(imcc, imcc->cur_unit, $3, 2, $1, $4); }
+ { $$ = MK_I(imcc, imcc->cur_unit, $3, 2, $1, $4);
+ $$->type = ITPUREFUNC; }
| target '=' var bin_op var
- { $$ = MK_I(imcc, imcc->cur_unit, $4, 3, $1, $3, $5); }
+ { $$ = MK_I(imcc, imcc->cur_unit, $4, 3, $1, $3, $5);
+ $$->type = ITPUREFUNC; }
| target '=' var '[' keylist ']'
{ $$ = iINDEXFETCH(imcc, imcc->cur_unit, $1, $3, $5); }
| target '[' keylist ']' '=' var
@@ -2023,6 +2026,7 @@ assignment:
| target '=' PNULL
{
$$ = MK_I(imcc, imcc->cur_unit, "null", 1, $1);
+ $$->type = ITPUREFUNC;
}
;
@@ -2074,13 +2078,15 @@ get_results:
op_assign:
target assign_op var
- { $$ = MK_I(imcc, imcc->cur_unit, $2, 2, $1, $3); }
+ { $$ = MK_I(imcc, imcc->cur_unit, $2, 2, $1, $3);
+ $$->type = ITPUREFUNC; }
| target CONCAT_ASSIGN var
{
if ($1->set == 'P')
$$ = MK_I(imcc, imcc->cur_unit, "concat", 2, $1, $3);
else
$$ = MK_I(imcc, imcc->cur_unit, "concat", 3, $1, $1, $3);
+ $$->type = ITPUREFUNC;
}
;
View
5 compilers/imcc/instructions.h
@@ -1,18 +1,19 @@
/*
- * Copyright (C) 2002-2010, Parrot Foundation.
+ * Copyright (C) 2002-2014, Parrot Foundation.
*/
#ifndef PARROT_IMCC_INSTRUCTIONS_H_GUARD
#define PARROT_IMCC_INSTRUCTIONS_H_GUARD
/* Types */
-enum INSTYPE { /*instruction type can be */
+enum INSTYPE { /* instruction type can be */
ITBRANCH = 0x10000, /* branch */
ITPCCRET = 0x20000, /* PCC sub return */
ITCALL = 0x40000, /* function call */
ITLABEL = 0x80000, /* label */
ITPCCPARAM = 0x100000, /* .get_params */
+ ITPUREFUNC = 0x200000, /* purely functional, no sideeffects */
ITRESULT = 0x400000, /* .get_results */
ITPCCSUB = 0x1000000, /* PCC sub call */
ITPCCYIELD = 0x2000000 /* yield from PCC call instead of return */
View
11 compilers/imcc/optimizer.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2002-2012, Parrot Foundation.
+ * Copyright (C) 2002-2014, Parrot Foundation.
*/
/*
@@ -277,7 +277,8 @@ cfg_optimize(ARGMOD(imc_info_t *imcc), ARGMOD(IMC_Unit *unit))
Runs after the CFG is built and handles constant propagation.
-used_once ... deletes assignments, when LHS is unused
+used_once ... deletes assignments, when LHS is unused and the
+op is purely functional, i.e. no side-effects.
=cut
@@ -723,7 +724,7 @@ constant_propagation(ARGMOD(imc_info_t *imcc), ARGMOD(IMC_Unit *unit))
tmp = IMCC_subst_constants(imcc,
unit, ins2->opname, ins2->symregs, ins2->opsize,
&found);
- if (found) {
+ if (found && tmp) { /* XXX syn/clash_1.pir */
const Instruction * const prev = ins2->prev;
if (prev) {
subst_ins(unit, ins2, tmp, 1);
@@ -1592,7 +1593,9 @@ used_once(ARGMOD(imc_info_t *imcc), ARGMOD(IMC_Unit *unit))
for (ins = unit->instructions; ins; ins = ins->next) {
if (ins->symregs) {
SymReg * const r = ins->symregs[0];
- if (r && (r->use_count == 1 && r->lhs_use_count == 1)) {
+ /* GH 1036: keep side-effects: P0 = pop P1 vs pop P1 */
+ if (r && ins->type & ITPUREFUNC
+ && (r->use_count == 1 && r->lhs_use_count == 1)) {
IMCC_debug(imcc, DEBUG_OPT2, "used once '%d' deleted\n", ins);
ins = delete_ins(unit, ins);
View
5 t/compilers/imcc/reg/alloc.t
@@ -1,9 +1,11 @@
#!perl
-# Copyright (C) 2005-2008, Parrot Foundation.
+# Copyright (C) 2005-2014, Parrot Foundation.
use strict;
use warnings;
use lib qw( . lib ../lib ../../lib );
+
+use Test::More;
use Parrot::Test tests => 10;
pir_output_is( <<'CODE', <<'OUT', "alligator" );
@@ -44,6 +46,7 @@ pir_output_is( <<'CODE', <<'OUT', "alligator 2 - r9629" );
$S0 = args[-1]
if $S0 != "POPME" goto start
+ # P0 used_once falsely deleted [GH #1036]
$P0 = pop args
start:
$I1 = elements args
View
4 t/compilers/imcc/reg/spill_old.t
@@ -1,9 +1,11 @@
#!perl
-# Copyright (C) 2001-2008, Parrot Foundation.
+# Copyright (C) 2001-2014, Parrot Foundation.
use strict;
use warnings;
use lib qw( . lib ../lib ../../lib );
+
+use Test::More;
use Parrot::Test tests => 5;
pir_output_is( <<'CODE', <<'OUT', "bug #32996" );
View
3 t/op/calling.t
@@ -1670,7 +1670,8 @@ ok 2
OUTPUT
TODO: {
- local $TODO = 'bad -O1 test 63' if $ENV{TEST_PROG_ARGS} =~ / -O[12]/;
+ local $TODO = 'bad -O1 test 63' if $ENV{TEST_PROG_ARGS}
+ and $ENV{TEST_PROG_ARGS} =~ / -O[12]/;
pir_output_is( <<'CODE', <<'OUTPUT', "newclosure followed by tailcall" );
## regression test for newclosure followed by tailcall, which used to recycle
## the context too soon. it looks awful because (a) the original version was

0 comments on commit 74055e5

Please sign in to comment.