From 9713796abe30c441f34d9b28d0582ce6f61002fb Mon Sep 17 00:00:00 2001 From: Luc Van Oostenryck Date: Sun, 7 Mar 2021 15:01:35 +0100 Subject: ssa: fix conversion with mismatched size or offset The SSA conversion works under the assumption that all the memory operations on a given symbol always refer to the same object. So, exclude the conversion of variables where: * memory operations do not always match in size or offset * there is an implicit integer/float conversion. Signed-off-by: Luc Van Oostenryck --- linearize.h | 2 +- ssa.c | 97 +++++++++++++++++++++++++++++------- validation/mem2reg/not-same-memop0.c | 1 - 3 files changed, 80 insertions(+), 20 deletions(-) diff --git a/linearize.h b/linearize.h index 7909b01f..86ae119c 100644 --- a/linearize.h +++ b/linearize.h @@ -18,7 +18,7 @@ struct pseudo_user { DECLARE_ALLOCATOR(pseudo_user); DECLARE_PTR_LIST(pseudo_user_list, struct pseudo_user); -DECLARE_PTRMAP(phi_map, struct symbol *, pseudo_t); +DECLARE_PTRMAP(phi_map, struct symbol *, struct instruction *); enum pseudo_type { diff --git a/ssa.c b/ssa.c index 26d46baa..f61672a0 100644 --- a/ssa.c +++ b/ssa.c @@ -96,10 +96,22 @@ static void kill_store(struct instruction *insn) insn->bb = NULL; } +static bool same_memop(struct instruction *a, struct instruction *b) +{ + if (a->size != b->size || a->offset != b->offset) + return false; + if (is_integral_type(a->type) && is_float_type(b->type)) + return false; + if (is_float_type(a->type) && is_integral_type(b->type)) + return false; + return true; +} + static void rewrite_local_var(struct basic_block *bb, pseudo_t addr, int nbr_stores, int nbr_uses) { + struct instruction *store = NULL; struct instruction *insn; - pseudo_t val = NULL; + bool remove = false; if (!bb) return; @@ -110,18 +122,21 @@ static void rewrite_local_var(struct basic_block *bb, pseudo_t addr, int nbr_sto continue; switch (insn->opcode) { case OP_LOAD: - if (!val) - val = undef_pseudo(); - replace_with_pseudo(insn, val); + if (!store) + replace_with_pseudo(insn, undef_pseudo()); + else if (same_memop(store, insn)) + replace_with_pseudo(insn, store->target); + else + remove = false; break; case OP_STORE: - val = insn->target; - // can't use kill_instruction() unless - // we add a fake user to val - kill_store(insn); + store = insn; + remove = true; break; } } END_FOR_EACH_PTR(insn); + if (remove) + kill_store(store); } static bool rewrite_single_store(struct instruction *store) @@ -139,6 +154,8 @@ static bool rewrite_single_store(struct instruction *store) // by the value from the store. This is only valid // if the store dominate the load. + if (!same_memop(store, insn)) + continue; if (insn->bb == store->bb) { // the load and the store are in the same BB // we can convert if the load is after the store. @@ -257,21 +274,40 @@ external_visibility: kill_dead_stores(ep, addr, !mod); } -static pseudo_t lookup_var(struct basic_block *bb, struct symbol *var) +static struct instruction *lookup_var(struct basic_block *bb, struct symbol *var) { do { - pseudo_t val = phi_map_lookup(bb->phi_map, var); - if (val) - return val; + struct instruction *insn = phi_map_lookup(bb->phi_map, var); + if (insn) + return insn; } while ((bb = bb->idom)); - return undef_pseudo(); + return NULL; } static struct instruction_list *phis_all; static struct instruction_list *phis_used; +static struct instruction_list *stores; + +static bool matching_load(struct instruction *def, struct instruction *insn) +{ + if (insn->size != def->size) + return false; + switch (def->opcode) { + case OP_STORE: + case OP_LOAD: + if (insn->offset != def->offset) + return false; + case OP_PHI: + break; + default: + return false; + } + return true; +} static void ssa_rename_insn(struct basic_block *bb, struct instruction *insn) { + struct instruction *def; struct symbol *var; pseudo_t addr; pseudo_t val; @@ -284,8 +320,8 @@ static void ssa_rename_insn(struct basic_block *bb, struct instruction *insn) var = addr->sym; if (!var || !var->torename) break; - phi_map_update(&bb->phi_map, var, insn->target); - kill_store(insn); + phi_map_update(&bb->phi_map, var, insn); + add_instruction(&stores, insn); break; case OP_LOAD: addr = insn->src; @@ -294,14 +330,22 @@ static void ssa_rename_insn(struct basic_block *bb, struct instruction *insn) var = addr->sym; if (!var || !var->torename) break; - val = lookup_var(bb, var); + def = lookup_var(bb, var); + if (!def) { + val = undef_pseudo(); + } else if (!matching_load(def, insn)) { + var->torename = false; + break; + } else { + val = def->target; + } replace_with_pseudo(insn, val); break; case OP_PHI: var = insn->type; if (!var || !var->torename) break; - phi_map_update(&bb->phi_map, var, insn->target); + phi_map_update(&bb->phi_map, var, insn); add_instruction(&phis_all, insn); break; } @@ -348,7 +392,8 @@ static void ssa_rename_phi(struct instruction *insn) return; FOR_EACH_PTR(insn->bb->parents, par) { struct instruction *term = delete_last_instruction(&par->insns); - pseudo_t val = lookup_var(par, var); + struct instruction *def = lookup_var(par, var); + pseudo_t val = def ? def->target : undef_pseudo(); pseudo_t phi = alloc_phi(par, val, var); phi->ident = var->ident; add_instruction(&par->insns, term); @@ -376,6 +421,18 @@ static void ssa_rename_phis(struct entrypoint *ep) } END_FOR_EACH_PTR(phi); } +static void remove_dead_stores(struct instruction_list *stores) +{ + struct instruction *store; + + FOR_EACH_PTR(stores, store) { + struct symbol *var = store->addr->sym; + + if (var->torename) + kill_store(store); + } END_FOR_EACH_PTR(store); +} + void ssa_convert(struct entrypoint *ep) { struct basic_block *bb; @@ -393,6 +450,7 @@ void ssa_convert(struct entrypoint *ep) } END_FOR_EACH_PTR(bb); // try to promote memory accesses to pseudos + stores = NULL; FOR_EACH_PTR(ep->accesses, pseudo) { ssa_convert_one_var(ep, pseudo->sym); } END_FOR_EACH_PTR(pseudo); @@ -401,4 +459,7 @@ void ssa_convert(struct entrypoint *ep) phis_all = phis_used = NULL; ssa_rename_insns(ep); ssa_rename_phis(ep); + + // remove now dead stores + remove_dead_stores(stores); } diff --git a/validation/mem2reg/not-same-memop0.c b/validation/mem2reg/not-same-memop0.c index 7a84ddce..4de98195 100644 --- a/validation/mem2reg/not-same-memop0.c +++ b/validation/mem2reg/not-same-memop0.c @@ -16,7 +16,6 @@ static void foo(struct s s) /* * check-name: not-same-memop0 * check-command: test-linearize -Wno-decl -fdump-ir=mem2reg $file - * check-known-to-fail * * check-output-start local: -- cgit 1.2.3-korg