# New Ticket Created by Angel Faus
# Please include the string: [perl #819]
# in the subject line of all future correspondence about this issue.
# <URL: http://bugs6.perl.org/rt2/Ticket/Display.html?id=819 >
Hi Melvin,
This patch does the following things:
- It includes patch #813 from Sean O'Rourke
- It fixes another bug in spill(), who was generating bad code
- It adds a bunch of work using the control-flow graph, analyzing the
exact places of the code where a variable is alive, and using this in
order to improve the interference analysis.
Incidentally, while tracking a memory bug, I added code to check the
results of all malloc calls. I've left it there, since it shouldn't
hurt.
I am sending you this through bugs-parrot, as people on the IRC
channel suggested.
Best,
-�ngel
[EMAIL PROTECTED]
-- attachment 1 ------------------------------------------------------
url: http://bugs6.perl.org/rt2/attach/3858/3565/a26fc6/patch_cfg.diff
-- attachment 2 ------------------------------------------------------
url: http://bugs6.perl.org/rt2/attach/3858/3566/337423/test_cfg.imc
RCS file: /cvs/public/parrot/languages/imcc/cfg.c,v
retrieving revision 1.1
diff -r1.1 cfg.c
28c28
<
---
>
30a31
> ins = NULL;
41,42c42,45
< bb->end = ins;
< bb = make_basic_block(next);
---
> if (ins != NULL) {
> bb->end = ins;
> bb = make_basic_block(next);
> }
121a125,128
> if (e==NULL) {
> fprintf(stderr, "Memory error at bb_add_edge\n");
> abort();
> }
131a139,310
>
> void life_analysis() {
> int i;
>
> for(i = 0; i < HASH_SIZE; i++) {
> SymReg * r = hash[i];
> for(; r; r = r->next) {
> if (r->type == VTIDENTIFIER)
> analyse_life_symbol(r);
> }
> }
> }
>
> void analyse_life_symbol(SymReg* r) {
> int i;
>
> r->life_info = calloc(n_basic_blocks, sizeof(Life_range*));
> if (r->life_info == NULL) {
> fprintf(stderr, "Memory error at analyse_life_symbol");
> abort();
> }
>
> /* First we make a pass to each block to gather the information
> * that can be obtained locally */
>
> for (i=0; i < n_basic_blocks; i++) {
> analyse_life_block(bb_list[i], r);
> }
>
> /* Now we need to consider the relations between blocks */
>
> for (i=0; i < n_basic_blocks; i++) {
> if (r->life_info[i]->flags & LF_use) {
>
> /* This block uses r, so it must be live at
> the beggining */
> r->life_info[i]->flags |= LF_lv_in;
>
> /* propagate this info to every predecessor */
> propagate_need (bb_list[i], r);
> }
> }
> }
>
> /* analyse_life_block studies the state of the var r
> * in the block bb.
> *
> * Its job is to set the flags LF_use, or LF_read,
> * and record the intervals inside the block where
> * the var is alive.
> */
>
> void analyse_life_block(Basic_block* bb, SymReg* r) {
> int i, write_pos, read_pos;
> Instruction* ins;
> Life_range* l;
>
> l = make_life_range(r, bb->index);
> write_pos = -1;
> read_pos = -1;
>
> for (i=bb->start->index; i < bb->end->index; i++) {
> ins = instructions[i];
> if (ins==NULL) {
> fprintf(stderr, "Index %i of %i has NULL instruction\n",
> i, bb->end->index);
> fprintf(stderr, "Total numb. of instructions = %li\n",
> n_instructions);
> abort();
> }
> if (instruction_reads(ins, r)) {
> if (l->flags != LF_def) {
>
> /* we read before having written before, so the var was
> * live at the beggining of the block */
> write_pos = bb->start->index;
> l->flags = LF_use;
> }
> read_pos = i;
> }
>
> if (instruction_writes(ins, r)) {
> if (write_pos < 0) {
> l->flags = LF_def;
> write_pos = i;
> }
> else if (read_pos < 0) {
> /* there has not been any read until here, so the previous write
> * is irrelevant */
> write_pos = i;
> }
> else {
> /* this is new writing, after some reading */
> add_life_interval(l, write_pos, read_pos);
> read_pos = -1;
> write_pos = i;
> }
> }
> }
>
> /* At the end, we need to add the last range */
> if (read_pos < 0) read_pos = write_pos;
>
> if (write_pos >= 0)
> add_life_interval(l, write_pos, read_pos);
>
>
> /* The read_pos can latter be extended if it turns out
> * that another block needs the value resulting of this
> * computation */
> }
>
> /* add_life_interval records a new range of use of the var
> * and set the LF_lv_inside flag
> */
>
> void add_life_interval(Life_range *l, int from, int to) {
> int length = l->n_intervals;
>
> l->intervals = realloc(l->intervals, (length + 2) * sizeof(int) + 1);
> if (l->intervals == NULL) {
> fprintf(stderr, "Memory error at add_life_interval\n");
> abort();
> }
>
> l->intervals[length*2] = from;
> l->intervals[length*2 + 1] = to;
>
> l->n_intervals = length + 1;
>
> l->flags |= LF_lv_inside;
>
> }
>
>
> void propagate_need(Basic_block *bb, SymReg* r) {
> Edge *edge;
> Basic_block *pred;
> Life_range *l;
>
> /* every predecessor of a LF_lv_in block must be in LF_lv_out
> and, unless itself is LV_def, this should be propagated to
> its predecessors themselves */
>
> for (edge=bb->pred_list; edge!=NULL; edge=edge->pred_next) {
> pred = edge->from;
> l = r->life_info[pred->index];
>
> if (l->flags & LF_lv_out) {
> /* this node has already been visited. Ignore it */
> }
> else {
> l->flags |= LF_lv_out;
>
> if (l->flags & LF_lv_inside) {
> /* we expand the last interval to the end */
> l->intervals[l->n_intervals * 2 - 1] = pred->end->index;
> }
>
> if (! (l->flags & LF_def) ) {
> l->flags |= LF_lv_in;
> if (! (l->flags & LF_lv_inside) ) {
> l->flags |= LF_lv_all | LF_lv_inside;
> }
>
> propagate_need(pred, r);
> }
> }
> }
> }
>
>
136,137c315,316
< if (bb_list == NULL) {
< free(bb_list);
---
> if (bb_list != NULL) {
> free(bb_list);
157a337,340
> if (bb==NULL) {
> fprintf(stderr, "Memory error at make_basic_block\n");
> abort();
> }
171a355,370
> Life_range* make_life_range(SymReg *r, int index) {
> Life_range *l;
>
> l = malloc(sizeof(Life_range));
> if (l==NULL) {
> fprintf(stderr, "Memory Error at make_life_range\n");
> abort();
> }
>
> l->flags = 0;
> l->n_intervals = 0;
> l->intervals = NULL;
>
> r->life_info[index] = l;
> return l;
> }
Index: cfg.h
===================================================================
RCS file: /cvs/public/parrot/languages/imcc/cfg.h,v
retrieving revision 1.1
diff -r1.1 cfg.h
23d22
<
35a35,40
> void life_analysis();
> void analyse_life_symbol(SymReg*);
> void analyse_life_block(Basic_block*, SymReg*);
> void add_life_interval(Life_range*, int, int);
> void propagate_need(Basic_block*, SymReg*);
>
38a44
> Life_range* make_life_range(SymReg*, int);
Index: debug.c
===================================================================
RCS file: /cvs/public/parrot/languages/imcc/debug.c,v
retrieving revision 1.1
diff -r1.1 debug.c
15c15
< fprintf(stderr, "%d\t", bb->index);
---
> fprintf(stderr, "%i\t%d\t", ins->index, bb->index);
60a61,104
> dump_liveness_status();
>
> }
>
> void dump_liveness_status() {
> int i;
>
> fprintf(stderr, "\nSymbols:\n--------------------------------------\n");
> for(i = 0; i < HASH_SIZE; i++) {
> SymReg * r = hash[i];
> for(; r; r = r->next) {
> if (r->type == VTIDENTIFIER) dump_liveness_status_var(r);
> }
> }
> fprintf(stderr, "\n");
>
> }
>
>
> void dump_liveness_status_var(SymReg* r) {
> int i, j;
> Life_range *l;
>
> fprintf(stderr, "\nSymbol %s:", r->name);
> if (r->life_info==NULL) return;
> for (i=0; i<n_basic_blocks; i++) {
> l = r->life_info[i];
>
> if (l->flags & LF_lv_all) {
> fprintf(stderr, "\n\t%i:ALL\t", i);
> }
> else if (l->flags & LF_lv_inside) {
> fprintf(stderr, "\n\t%i:", i);
>
> if (l->flags & LF_lv_in) fprintf(stderr, "IN\t");
> if (l->flags & LF_lv_out) fprintf(stderr, "OUT\t");
>
> for (j=0; j < l->n_intervals; j++) {
> fprintf(stderr, "[%d,%d]\t",
> l->intervals[2*j], l->intervals[2*j+1] );
> }
> }
> }
> fprintf(stderr, "\n");
64c108
< int x, y, cnt;
---
> int x, y, cnt;
Index: debug.h
===================================================================
RCS file: /cvs/public/parrot/languages/imcc/debug.h,v
retrieving revision 1.1
diff -r1.1 debug.h
5a6,7
> void dump_liveness_status();
> void dump_liveness_status_var();
Index: imc.c
===================================================================
RCS file: /cvs/public/parrot/languages/imcc/imc.c,v
retrieving revision 1.9
diff -r1.9 imc.c
41c41
< /*life_analysis();*/
---
> life_analysis();
98c98,103
< for(i = 0; i < HASH_SIZE; i++) {
---
> if (interference_graph == NULL) {
> fprintf(stderr, "Memory error at build_interference_graph\n");
> abort();
> }
>
> for(i = 0; i < HASH_SIZE; i++) {
123c128
< dump_interference_graph();
---
> /*dump_interference_graph();*/
149a155,159
> /* We currently decide that two vars interfere if they are both alive
> * at any point. This could be improved, requiring that one is alive
> * at the point of _definition_ of the other.
> */
>
150a161,162
> int i;
>
170a183,211
> /* Now: */
>
> for (i=0; i <n_basic_blocks; i++) {
> Life_range *l0, *l1;
>
> if ( (l0->flags & LF_lv_all && l1->flags & LF_lv_inside)
> || (l0->flags & LF_lv_inside && l1->flags & LF_lv_all)
> || (l0->flags & LF_lv_in && l1->flags & LF_lv_in)
> || (l0->flags & LF_lv_out && l1->flags & LF_lv_out)
> )
> return 1;
>
> if (l0->flags & LF_lv_inside && l1->flags & LF_lv_inside) {
> /* we need to compare the intervals */
> int i, j;
> for (i=0; i < l0->n_intervals; i++) {
> for (j=0; j < l1->n_intervals; j++) {
> if (l0->intervals[i] >= l1->intervals[j+1] )
> continue;
>
> if (l0->intervals[i+1] <= l1->intervals[j] )
> continue;
>
> return 1;
> }
> }
> }
> }
>
183c224
< int simplify (SymReg **g){
---
> int simplify (){
185a227
> SymReg **g;
186a229,230
> g = interference_graph;
>
191c235
<
---
>
345,346c389,390
< Instruction ** new_instructions;
<
---
> Instruction ** old_instructions;
> int old_n_instructions;
348,349c392,394
< int i, j, needs_fetch, needs_store, needs_spilling, after_spilled, after_needs_store;
< SymReg *new_symbol, *old_symbol;
---
> int i, j;
> int needs_fetch, needs_store, needs_spilling, after_spilled, after_needs_store;
> SymReg *new_symbol, *old_symbol;
352d396
< SymReg **g = interference_graph;
353a398,401
> if (buf == NULL) {
> fprintf(stderr, "Memory error at spill\n");
> abort();
> }
356c404
< fprintf(stderr, "#Spilling [%s]:\n", g[spilled]->name);
---
> fprintf(stderr, "#Spilling [%s]:\n", interference_graph[spilled]->name);
358c406,409
< old_symbol = g[spilled];
---
> old_symbol = interference_graph[spilled];
> old_instructions = instructions;
> old_n_instructions = n_instructions;
> instructions = NULL;
362d412
< new_instructions = calloc(4096, sizeof(Instruction *));
369,370c419,420
< for(i = 0; i < n_instructions; i++) {
< tmp = instructions[i];
---
> for(i = 0; i < old_n_instructions; i++) {
> tmp = old_instructions[i];
375,403c425,426
< if (tmp->r0 == old_symbol) {
< if (tmp->flags & IF_r0_read) needs_fetch = 1;
< if (tmp->flags & IF_r0_write) needs_store = 1;
<
< tmp->r0 = new_symbol;
< }
<
< if (tmp->r1 == old_symbol) {
< if (tmp->flags & IF_r1_read) needs_fetch = 1;
< if (tmp->flags & IF_r1_write) needs_store = 1;
<
< tmp->r1 = new_symbol;
< }
<
<
< if (tmp->r2 == old_symbol) {
< if (tmp->flags & IF_r2_read) needs_fetch = 1;
< if (tmp->flags & IF_r2_write) needs_store = 1;
<
< tmp->r2 = new_symbol;
< }
<
<
< if (tmp->r3 == old_symbol) {
< if (tmp->flags & IF_r3_read) needs_fetch = 1;
< if (tmp->flags & IF_r3_write) needs_store = 1;
<
< tmp->r3 = new_symbol;
< }
---
> if (instruction_reads (tmp, old_symbol) )
> needs_fetch = 1;
404a428,430
> if (instruction_writes (tmp, old_symbol) )
> needs_store = 1;
>
409,413c435,436
< sprintf(buf, "set %s, P31, %d #FETCH", "%s", n_spilled); /*ouch*/
<
< new_instructions[j++] = mk_instruction(
< buf, new_symbol, NULL, NULL, NULL, IF_r1_write);
<
---
> sprintf(buf, "set %s, P31[%d], #FETCH", "%s", n_spilled); /*ouch*/
> emitb( mk_instruction(buf, new_symbol, NULL, NULL, NULL, IF_r1_write));
418,421c441,442
< sprintf(buf, "set P31, %d, %s #STORE", n_spilled, "%s"); /*ouch, ouch*/
<
< new_instructions[j++] = mk_instruction(
< buf, new_symbol, NULL, NULL, NULL, IF_r1_write);
---
> sprintf(buf, "set P31[%d], %s #STORE", n_spilled, "%s"); /*ouch, ouch*/
> emitb ( mk_instruction(buf, new_symbol, NULL, NULL, NULL, IF_r1_read));
424c445
< new_symbol = mk_symreg(buf, old_symbol->set);
---
> new_symbol = mk_ident(buf, old_symbol->set);
427c448,463
< new_instructions[j++] = tmp;
---
> /* Emit the old instruction, with the symbol changed */
> {
> SymReg *r0, *r1, *r2, *r3;
>
> r0 = tmp->r0;
> r1 = tmp->r1;
> r2 = tmp->r2;
> r3 = tmp->r3;
>
> if (r0==old_symbol) r0=new_symbol;
> if (r1==old_symbol) r1=new_symbol;
> if (r2==old_symbol) r2=new_symbol;
> if (r3==old_symbol) r3=new_symbol;
>
> emitb( mk_instruction(tmp->fmt, r0, r1, r2, r3, tmp->flags) );
> }
432,434c468
< free(instructions);
< instructions = new_instructions;
< n_instructions = j;
---
> free(old_instructions);
441a476,477
>
> dump_instructions();
467a504,507
> if (copy == NULL) {
> fprintf(stderr, "Memory error at str_dup\n");
> abort();
> }
474a515,518
> if (s3 == NULL) {
> fprintf(stderr, "Memory error at str_cat\n");
> abort();
> }
Index: imc.h
===================================================================
RCS file: /cvs/public/parrot/languages/imcc/imc.h,v
retrieving revision 1.8
diff -r1.8 imc.h
39c39
< int n_spill;
---
> int n_spilled;
Index: instructions.c
===================================================================
RCS file: /cvs/public/parrot/languages/imcc/instructions.c,v
retrieving revision 1.1
diff -r1.1 instructions.c
31a32,35
> if (i == NULL) {
> fprintf(stderr, "Memory error at mk_instruction\n");
> abort();
> }
51a56,92
> int instruction_reads(Instruction* ins, SymReg* r) {
> int f;
>
> if (ins == NULL) {
> fprintf(stderr, "Internal error: instruction_reads called with NULL argument\n");
> abort();
> }
>
> f = ins->flags;
>
> if ((ins->r0 == r) && f & IF_r0_read) return 1;
> if ((ins->r1 == r) && f & IF_r1_read) return 1;
> if ((ins->r2 == r) && f & IF_r2_read) return 1;
> if ((ins->r3 == r) && f & IF_r3_read) return 1;
>
> return 0;
> }
>
> int instruction_writes(Instruction* ins, SymReg* r) {
> int f;
>
> if (ins == NULL) {
> fprintf(stderr, "Internal error: instruction_reads called with NULL argument\n");
> abort();
> }
>
> f = ins->flags;
>
> if ((ins->r0 == r) && f & IF_r0_write) return 1;
> if ((ins->r1 == r) && f & IF_r1_write) return 1;
> if ((ins->r2 == r) && f & IF_r2_write) return 1;
> if ((ins->r3 == r) && f & IF_r3_write) return 1;
>
> return 0;
> }
>
>
55a97,99
> if (i == NULL) {
> fprintf(stderr, "Memory error at resize_instructions\n");
> }
67a112,115
> if (instructions == NULL) {
> fprintf(stderr, "Memory error at emitb\n");
> abort();
> }
70a119
> i->index = n_instructions;
73d121
< i->basic_block = NULL;
85c133
< if (n_spill > 0) {
---
> if (n_spilled > 0) {
Index: instructions.h
===================================================================
RCS file: /cvs/public/parrot/languages/imcc/instructions.h,v
retrieving revision 1.1
diff -r1.1 instructions.h
17c17
< long flags; /* how the instruction affects each of the values */
---
> long flags; /* how the instruction affects each of the values */
19c19,20
< void * basic_block; /* basic block */
---
> void * basic_block; /* basic block */
> int index; /* index on instructions[] */
50a52,53
> int instruction_reads(Instruction *, SymReg *);
> int instruction_writes(Instruction *, SymReg *);
Index: symreg.c
===================================================================
RCS file: /cvs/public/parrot/languages/imcc/symreg.c,v
retrieving revision 1.1
diff -r1.1 symreg.c
19a20,23
> if (r==NULL) {
> fprintf(stderr, "Memory error at mk_symreg\n");
> abort();
> }
31a36
> r->life_info = NULL;
50a56,59
> if (r==NULL) {
> fprintf(stderr, "Memory error at mk_const\n");
> abort();
> }
58a68
> r->life_info = NULL;
69a80,83
> if (r==NULL) {
> fprintf(stderr, "Memory error at mk_address\n");
> abort();
> }
75a90
> r->life_info = NULL;
Index: symreg.h
===================================================================
RCS file: /cvs/public/parrot/languages/imcc/symreg.h,v
retrieving revision 1.1
diff -r1.1 symreg.h
15a16,33
> enum LIFEFLAG { /* The status of a var inside a basic block can be */
> LF_use = 1 << 0, /* block uses the the var before defining it */
> LF_def = 1 << 1, /* block defines the variable */
> LF_lv_in = 1 << 2, /* variable is alive at the beggining of the block */
> LF_lv_out = 1 << 3, /* variable is alive at the end of the block */
> LF_lv_inside = 1 << 4, /* variable is alive at some momement in the block */
> LF_lv_all = 1 << 5 /* alive during all the block */
> };
>
> /* Liveness represents the usage of a var inside a basic block
> This is represented by pairs of [definition, usage] in *intervals:
> */
> typedef struct _Life_range {
> int flags;
> int n_intervals;
> int *intervals;
> } Life_range;
>
27a46
> Life_range **life_info; /* Each block has its Life_range status */
30d48
<
## COMPUTES the m-th Fibonacci number.
..sub _MAIN
.local int f0
.local int f1
.local int f2
.local int m
.local int i
f0 = 1
f1 = 1
m = 30
if m <= 1 goto LBL3
i = 2
LBL1:
if i > m goto LBL2
f2 = f0 + f1
f0 = f1
f1 = f2
i = i + 1
goto LBL1
LBL3:
print m
goto RET
LBL2:
print f2
goto RET
RET:
print "\n"
end
ret