| Message ID | alpine.LNX.2.00.1006281243490.1429@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
On 06/28/2010 12:58 PM, Richard Guenther wrote: > INDIRECT_REF is not allowed in gimple. You can have bare decls or > MEM_REF, and both can be wrapped inside a handled_component_p chain. > (There also still exist ALIGN_INDIRECT_REF and MISALIGNED_INDIRECT_REF > created by the vectorizer - I will do followup patches to remove those > as well) > > Basically MEM_REF can appear wherever an INDIRECT_REF or a decl could > previously apper. Thanks, this should likely go in the docs. >> Would it make sense to change the other INDIRECT_REF_P types into flags on >> MEM_REF? > > INDIRECT_REF_P will be no longer necessary on gimple after I fixed > up the vectorizer to not use ALIGN_INDIRECT_REF and > MISALIGNED_INDIRECT_REF. Where it currently appears it is solely > because of those two (I didn't bother to change it to > tests for ALIGN_INDIRECT_REF and MISALIGNED_INDIRECT_REF excluding > INDIRECT_REF). Then I had understood correctly, thanks. :) Any idea about the future representation of ALIGN_INDIRECT_REF and MISALIGNED_INDIRECT_REF? >> Can you expand on the (several) "#if 0/FIXME" comments? > > They should have been fixed by followup patches (I just noticed them > while writing the changelog). Yes, indeed. Paolo
On Mon, 28 Jun 2010, Paolo Bonzini wrote: > On 06/28/2010 12:58 PM, Richard Guenther wrote: > > INDIRECT_REF is not allowed in gimple. You can have bare decls or > > MEM_REF, and both can be wrapped inside a handled_component_p chain. > > (There also still exist ALIGN_INDIRECT_REF and MISALIGNED_INDIRECT_REF > > created by the vectorizer - I will do followup patches to remove those > > as well) > > > > Basically MEM_REF can appear wherever an INDIRECT_REF or a decl could > > previously apper. > > Thanks, this should likely go in the docs. I'm going to search for a proper place ... > > > Would it make sense to change the other INDIRECT_REF_P types into flags on > > > MEM_REF? > > > > INDIRECT_REF_P will be no longer necessary on gimple after I fixed > > up the vectorizer to not use ALIGN_INDIRECT_REF and > > MISALIGNED_INDIRECT_REF. Where it currently appears it is solely > > because of those two (I didn't bother to change it to > > tests for ALIGN_INDIRECT_REF and MISALIGNED_INDIRECT_REF excluding > > INDIRECT_REF). > > Then I had understood correctly, thanks. :) Any idea about the future > representation of ALIGN_INDIRECT_REF and MISALIGNED_INDIRECT_REF? ALIGN_INDIRECT_REF I will replace with ptr = ptr & mask; MEM_REF [ptr]; thus simply do what expansion replaces it with. That will allow straight-forward tracking of alignment via ... ... the replacement for MISALIGNED_INDIRECT_REF. We have the long-standing problem of not being able to track alignment or conservatively assess that of pointers. My plan is to embed alignment information in SSA_NAME_PTR_INFO, an alignment value plus a misalignment value. Thus, double x; char *foo(void) { char *p = &x; alignment 8, misalign 0 p++; alignment 8, misalign 1 p++; alignment 8, misalign 2 ... } information filled in by a simple propagator pass. I'm not yet sure whether to piggy-back on an existing pass or whether to invent a new one. Into-SSA will fill in information from dereference sites - we will still rely on the frontends to provide correct pointed-to alignments. So for ... = MEM [(char *)p, 2] we will conclude that x is aligned to a byte boundary while for ... = MEM [(int *)p, 2] we will conclude that x is aligned to 4 bytes with misalignment 2. Similar to what we derive from INDIRECT_REF types at expansion time. Like when deriving information from INDIRECT_REFs this is somewhat fragile, so this is only to be done on unoptimized code during into-SSA. Basically all the code looking at types in get_pointer_alignment and set_mem_attributes_minus_bitpos is supposed to vanish and alignment is to be derived from the SSA_NAME_PTR_INFO only. Richard.
> On Mon, 28 Jun 2010, Paolo Bonzini wrote: > > > On 06/28/2010 12:58 PM, Richard Guenther wrote: > > > INDIRECT_REF is not allowed in gimple. You can have bare decls or > > > MEM_REF, and both can be wrapped inside a handled_component_p chain. > > > (There also still exist ALIGN_INDIRECT_REF and MISALIGNED_INDIRECT_REF > > > created by the vectorizer - I will do followup patches to remove those > > > as well) > > > > > > Basically MEM_REF can appear wherever an INDIRECT_REF or a decl could > > > previously apper. > > > > Thanks, this should likely go in the docs. > > I'm going to search for a proper place ... > > > > > Would it make sense to change the other INDIRECT_REF_P types into flags on > > > > MEM_REF? > > > > > > INDIRECT_REF_P will be no longer necessary on gimple after I fixed > > > up the vectorizer to not use ALIGN_INDIRECT_REF and > > > MISALIGNED_INDIRECT_REF. Where it currently appears it is solely > > > because of those two (I didn't bother to change it to > > > tests for ALIGN_INDIRECT_REF and MISALIGNED_INDIRECT_REF excluding > > > INDIRECT_REF). > > > > Then I had understood correctly, thanks. :) Any idea about the future > > representation of ALIGN_INDIRECT_REF and MISALIGNED_INDIRECT_REF? > > ALIGN_INDIRECT_REF I will replace with > > ptr = ptr & mask; > MEM_REF [ptr]; > > thus simply do what expansion replaces it with. That will allow > straight-forward tracking of alignment via ... > > ... the replacement for MISALIGNED_INDIRECT_REF. We have the > long-standing problem of not being able to track alignment > or conservatively assess that of pointers. My plan is to > embed alignment information in SSA_NAME_PTR_INFO, an > alignment value plus a misalignment value. Thus, > > double x; > > char *foo(void) > { > char *p = &x; alignment 8, misalign 0 > p++; alignment 8, misalign 1 > p++; alignment 8, misalign 2 > ... > } This is definitly important for stringop expansion, especially when we can track the alignment returned from malloc to be a lot greater than what type suggest... Honza
On Mon, 28 Jun 2010, Richard Guenther wrote: > On Mon, 28 Jun 2010, Paolo Bonzini wrote: > > > On 06/25/2010 01:47 PM, Richard Guenther wrote: > > > + if (integer_zerop (TREE_OPERAND (node, 1)) > > > + /* Same pointer types, but ignoring POINTER_TYPE vs. > > > + REFERENCE_TYPE. */ > > > + && (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 0))) > > > + == TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1)))) > > > + && (TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 0))) > > > + == TYPE_MODE (TREE_TYPE (TREE_OPERAND (node, 1)))) > > > + && (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, 0))) > > > + == TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (node, > > > 1)))) > > > + && (TYPE_QUALS (TREE_TYPE (TREE_OPERAND (node, 0))) > > > + == TYPE_QUALS (TREE_TYPE (TREE_OPERAND (node, 1)))) > > > + /* Same value types ignoring qualifiers. */ > > > + && (TYPE_MAIN_VARIANT (TREE_TYPE (node)) > > > + == TYPE_MAIN_VARIANT > > > + (TREE_TYPE (TREE_TYPE (TREE_OPERAND (node, 1)))))) > > > + { > > > + if (TREE_CODE (TREE_OPERAND (node, 0)) != ADDR_EXPR) > > > + { > > > + pp_string (buffer, "*"); > > > + dump_generic_node (buffer, TREE_OPERAND (node, 0), > > > + spc, flags, false); > > > + } > > > > Maybe avoid the magic when detailed dumping is active? > > The magic is mostly to make the testsuite part of the patch smaller, > which also means that if we change the above with -details some > testcases may start failing. I'll check, the suggestion is good > I think (so if the resulting change in testresults is small I'll > probably go for it). On a 2nd thought -details isn't really suitable for this. If in the end people do not feel too distracted when seeing 'MEM[(int *)&a]' instead of 'a' then I'd rather disable the above fancy-printing completely (and adjust all testcases). As a followup after the merge. Richard.
On 06/28/2010 02:38 PM, Richard Guenther wrote: > ALIGN_INDIRECT_REF I will replace with > > ptr = ptr & mask; > MEM_REF [ptr]; > > thus simply do what expansion replaces it with. So you are relying on TER to put together the pieces, right (when it makes sense)? Furthermore, the MEM_REF will have -mask alignment thanks to the MISALIGNED_INDIRECT_REF infrastructure, and the misalignment info may in the future allow replacing the & with an offset in the MEM_REF. Makes sense, thanks! Regarding the dumping question, it definitely can wait past merge, no matter how it is implemented. Paolo
On Mon, 28 Jun 2010, Paolo Bonzini wrote: > On 06/28/2010 02:38 PM, Richard Guenther wrote: > > ALIGN_INDIRECT_REF I will replace with > > > > ptr = ptr & mask; > > MEM_REF [ptr]; > > > > thus simply do what expansion replaces it with. > > So you are relying on TER to put together the pieces, right (when it makes > sense)? Well, I'm just expanding it on trees early. In expr.c we have case MISALIGNED_INDIRECT_REF: case ALIGN_INDIRECT_REF: case INDIRECT_REF: { ... if (code == ALIGN_INDIRECT_REF) { int align = TYPE_ALIGN_UNIT (type); op0 = gen_rtx_AND (address_mode, op0, GEN_INT (-align)); op0 = memory_address_addr_space (mode, op0, as); } we seem to rely on combine or whatever to do fancy things (I tried to figure out where exactly things should be optimized but I even failed to see in which cases exactly we end up generating ALIGN_INDIRECT_REF in the first place ...) > Furthermore, the MEM_REF will have -mask alignment thanks to the > MISALIGNED_INDIRECT_REF infrastructure, and the misalignment info may in the > future allow replacing the & with an offset in the MEM_REF. Makes sense, > thanks! Right. > Regarding the dumping question, it definitely can wait past merge, no matter > how it is implemented. It's definitely useful for debugging. Richard.
On 06/28/2010 05:53 PM, Richard Guenther wrote: > On Mon, 28 Jun 2010, Paolo Bonzini wrote: > >> On 06/28/2010 02:38 PM, Richard Guenther wrote: >>> ALIGN_INDIRECT_REF I will replace with >>> >>> ptr = ptr& mask; >>> MEM_REF [ptr]; >>> >>> thus simply do what expansion replaces it with. >> >> So you are relying on TER to put together the pieces, right (when it makes >> sense)? > > Well, I'm just expanding it on trees early. In expr.c we have > > case MISALIGNED_INDIRECT_REF: > case ALIGN_INDIRECT_REF: > case INDIRECT_REF: > { > ... > if (code == ALIGN_INDIRECT_REF) > { > int align = TYPE_ALIGN_UNIT (type); > op0 = gen_rtx_AND (address_mode, op0, GEN_INT (-align)); > op0 = memory_address_addr_space (mode, op0, as); > } > > we seem to rely on combine or whatever to do fancy things No, the AND is getting into the expansion directly if it is a legitimate address. > (I tried > to figure out where exactly things should be optimized but I even > failed to see in which cases exactly we end up generating > ALIGN_INDIRECT_REF in the first place ...) I think PPC vectorization uses it. Paolo
On Mon, 28 Jun 2010, Paolo Bonzini wrote: > On 06/28/2010 05:53 PM, Richard Guenther wrote: > > On Mon, 28 Jun 2010, Paolo Bonzini wrote: > > > > > On 06/28/2010 02:38 PM, Richard Guenther wrote: > > > > ALIGN_INDIRECT_REF I will replace with > > > > > > > > ptr = ptr& mask; > > > > MEM_REF [ptr]; > > > > > > > > thus simply do what expansion replaces it with. > > > > > > So you are relying on TER to put together the pieces, right (when it makes > > > sense)? > > > > Well, I'm just expanding it on trees early. In expr.c we have > > > > case MISALIGNED_INDIRECT_REF: > > case ALIGN_INDIRECT_REF: > > case INDIRECT_REF: > > { > > ... > > if (code == ALIGN_INDIRECT_REF) > > { > > int align = TYPE_ALIGN_UNIT (type); > > op0 = gen_rtx_AND (address_mode, op0, GEN_INT (-align)); > > op0 = memory_address_addr_space (mode, op0, as); > > } > > > > we seem to rely on combine or whatever to do fancy things > > No, the AND is getting into the expansion directly if it is a legitimate > address. Which is a hack, right? With the plan I have in mind we'd have ptr &= 3; // align the pointer .. = *p; // aligned load which should expand fine (to an AND and to an aligned load). Then combine can combine those two insns just fine to the aligning load. > > (I tried > > to figure out where exactly things should be optimized but I even > > failed to see in which cases exactly we end up generating > > ALIGN_INDIRECT_REF in the first place ...) > > I think PPC vectorization uses it. Of course - that much I figured out by simple grepping ;) I just didn't figure out why people thought it was a great idea to invent ALIGN_INDIRECT_REF when a simple BIT_AND_EXPR on the tree-level would do (not?). Thus it's now my part to find out the hard way ... but maybe Ira has an idea? Thanks, Richard.
On 06/29/2010 11:03 AM, Richard Guenther wrote: >>> if (code == ALIGN_INDIRECT_REF) >>> { >>> int align = TYPE_ALIGN_UNIT (type); >>> op0 = gen_rtx_AND (address_mode, op0, GEN_INT (-align)); >>> op0 = memory_address_addr_space (mode, op0, as); >>> } >>> >>> we seem to rely on combine or whatever to do fancy things >> >> No, the AND is getting into the expansion directly if it is a legitimate >> address. > > Which is a hack, right? With the plan I have in mind we'd have > > ptr&= 3; // align the pointer > .. = *p; // aligned load > > which should expand fine (to an AND and to an aligned load). Then > combine can combine those two insns just fine to the aligning load. Not necessarily a hack; I could say that a BIT_AND_EXPR on a pointer, with a pointer-typed operand 1 0xFFFFFFF0, is also a hack... But if TER or fwprop can create the addresses already, that's fine by me. Paolo
On Tue, 29 Jun 2010, Richard Guenther wrote: > On Mon, 28 Jun 2010, Paolo Bonzini wrote: > > > On 06/28/2010 05:53 PM, Richard Guenther wrote: > > > On Mon, 28 Jun 2010, Paolo Bonzini wrote: > > > > > > > On 06/28/2010 02:38 PM, Richard Guenther wrote: > > > > > ALIGN_INDIRECT_REF I will replace with > > > > > > > > > > ptr = ptr& mask; > > > > > MEM_REF [ptr]; > > > > > > > > > > thus simply do what expansion replaces it with. > > > > > > > > So you are relying on TER to put together the pieces, right (when it makes > > > > sense)? > > > > > > Well, I'm just expanding it on trees early. In expr.c we have > > > > > > case MISALIGNED_INDIRECT_REF: > > > case ALIGN_INDIRECT_REF: > > > case INDIRECT_REF: > > > { > > > ... > > > if (code == ALIGN_INDIRECT_REF) > > > { > > > int align = TYPE_ALIGN_UNIT (type); > > > op0 = gen_rtx_AND (address_mode, op0, GEN_INT (-align)); > > > op0 = memory_address_addr_space (mode, op0, as); > > > } > > > > > > we seem to rely on combine or whatever to do fancy things > > > > No, the AND is getting into the expansion directly if it is a legitimate > > address. > > Which is a hack, right? With the plan I have in mind we'd have > > ptr &= 3; // align the pointer > .. = *p; // aligned load > > which should expand fine (to an AND and to an aligned load). Then > combine can combine those two insns just fine to the aligning load. > > > > (I tried > > > to figure out where exactly things should be optimized but I even > > > failed to see in which cases exactly we end up generating > > > ALIGN_INDIRECT_REF in the first place ...) > > > > I think PPC vectorization uses it. > > Of course - that much I figured out by simple grepping ;) I just > didn't figure out why people thought it was a great idea to > invent ALIGN_INDIRECT_REF when a simple BIT_AND_EXPR on the tree-level > would do (not?). > > Thus it's now my part to find out the hard way ... but maybe > Ira has an idea? Btw, the following hack at least passes the C vectorizer testsuite on powerpc64 (and no rliwnm instructions are generated). So I plan to continue down this road to eliminate ALIGN_INDIRECT_REF. Richard. Index: gcc/expr.c =================================================================== *** gcc/expr.c (revision 161488) --- gcc/expr.c (working copy) *************** expand_expr_real_1 (tree exp, rtx target *** 8677,8688 **** return expand_constructor (exp, target, modifier, false); case MISALIGNED_INDIRECT_REF: - case ALIGN_INDIRECT_REF: case INDIRECT_REF: { tree exp1 = treeop0; addr_space_t as = ADDR_SPACE_GENERIC; - enum machine_mode address_mode = Pmode; if (modifier != EXPAND_WRITE) { --- 8677,8686 ---- *************** expand_expr_real_1 (tree exp, rtx target *** 8696,8714 **** if (POINTER_TYPE_P (TREE_TYPE (exp1))) { as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp1))); - address_mode = targetm.addr_space.address_mode (as); } op0 = expand_expr (exp1, NULL_RTX, VOIDmode, EXPAND_SUM); op0 = memory_address_addr_space (mode, op0, as); - if (code == ALIGN_INDIRECT_REF) - { - int align = TYPE_ALIGN_UNIT (type); - op0 = gen_rtx_AND (address_mode, op0, GEN_INT (-align)); - op0 = memory_address_addr_space (mode, op0, as); - } - temp = gen_rtx_MEM (mode, op0); set_mem_attributes (temp, exp, 0); --- 8694,8704 ---- *************** expand_expr_real_1 (tree exp, rtx target *** 8773,8778 **** --- 8763,8769 ---- = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 1)))); enum machine_mode address_mode; tree base = TREE_OPERAND (exp, 0); + gimple def_stmt; /* Handle expansion of non-aliased memory with non-BLKmode. That might end up in a register. */ if (TREE_CODE (base) == ADDR_EXPR) *************** expand_expr_real_1 (tree exp, rtx target *** 8815,8822 **** } } address_mode = targetm.addr_space.address_mode (as); ! op0 = expand_expr (TREE_OPERAND (exp, 0), NULL_RTX, address_mode, ! EXPAND_NORMAL); if (!integer_zerop (TREE_OPERAND (exp, 1))) { rtx off; --- 8806,8817 ---- } } address_mode = targetm.addr_space.address_mode (as); ! base = TREE_OPERAND (exp, 0); ! if ((def_stmt = get_def_for_expr (base, BIT_AND_EXPR))) ! base = build2 (BIT_AND_EXPR, TREE_TYPE (base), ! gimple_assign_rhs1 (def_stmt), ! gimple_assign_rhs2 (def_stmt)); ! op0 = expand_expr (base, NULL_RTX, address_mode, EXPAND_NORMAL); if (!integer_zerop (TREE_OPERAND (exp, 1))) { rtx off; Index: gcc/tree-vect-data-refs.c =================================================================== *** gcc/tree-vect-data-refs.c (revision 161488) --- gcc/tree-vect-data-refs.c (working copy) *************** vect_setup_realignment (gimple stmt, gim *** 3178,3184 **** vec_dest = vect_create_destination_var (scalar_dest, vectype); ptr = vect_create_data_ref_ptr (stmt, loop_for_initial_load, NULL_TREE, &init_addr, &inc, true, &inv_p); ! data_ref = build1 (ALIGN_INDIRECT_REF, vectype, ptr); new_stmt = gimple_build_assign (vec_dest, data_ref); new_temp = make_ssa_name (vec_dest, new_stmt); gimple_assign_set_lhs (new_stmt, new_temp); --- 3178,3192 ---- vec_dest = vect_create_destination_var (scalar_dest, vectype); ptr = vect_create_data_ref_ptr (stmt, loop_for_initial_load, NULL_TREE, &init_addr, &inc, true, &inv_p); ! new_stmt = gimple_build_assign_with_ops (BIT_AND_EXPR, ! NULL_TREE, ptr, ! build_int_cst (TREE_TYPE (ptr), ! -(HOST_WIDE_INT)TYPE_ALIGN_UNIT ((vectype)))); ! new_temp = make_ssa_name (SSA_NAME_VAR (ptr), new_stmt); ! gimple_assign_set_lhs (new_stmt, new_temp); ! new_bb = gsi_insert_on_edge_immediate (pe, new_stmt); ! gcc_assert (!new_bb); ! data_ref = build_simple_mem_ref (new_temp); new_stmt = gimple_build_assign (vec_dest, data_ref); new_temp = make_ssa_name (vec_dest, new_stmt); gimple_assign_set_lhs (new_stmt, new_temp); Index: gcc/tree-vect-stmts.c =================================================================== *** gcc/tree-vect-stmts.c (revision 161488) --- gcc/tree-vect-stmts.c (working copy) *************** vectorizable_load (gimple stmt, gimple_s *** 3684,3690 **** dr_explicit_realign, dataref_ptr, NULL); ! data_ref = build1 (ALIGN_INDIRECT_REF, vectype, dataref_ptr); vec_dest = vect_create_destination_var (scalar_dest, vectype); new_stmt = gimple_build_assign (vec_dest, data_ref); new_temp = make_ssa_name (vec_dest, new_stmt); --- 3684,3697 ---- dr_explicit_realign, dataref_ptr, NULL); ! new_stmt = gimple_build_assign_with_ops (BIT_AND_EXPR, ! NULL_TREE, dataref_ptr, ! build_int_cst (TREE_TYPE (dataref_ptr), ! -(HOST_WIDE_INT)TYPE_ALIGN_UNIT ((vectype)))); ! ptr = make_ssa_name (SSA_NAME_VAR (dataref_ptr), new_stmt); ! gimple_assign_set_lhs (new_stmt, ptr); ! vect_finish_stmt_generation (stmt, new_stmt, gsi); ! data_ref = build_simple_mem_ref (ptr); vec_dest = vect_create_destination_var (scalar_dest, vectype); new_stmt = gimple_build_assign (vec_dest, data_ref); new_temp = make_ssa_name (vec_dest, new_stmt); *************** vectorizable_load (gimple stmt, gimple_s *** 3697,3707 **** bump = size_binop (MULT_EXPR, vs_minus_1, TYPE_SIZE_UNIT (scalar_type)); ptr = bump_vector_ptr (dataref_ptr, NULL, gsi, stmt, bump); ! data_ref = build1 (ALIGN_INDIRECT_REF, vectype, ptr); break; } case dr_explicit_realign_optimized: ! data_ref = build1 (ALIGN_INDIRECT_REF, vectype, dataref_ptr); break; default: gcc_unreachable (); --- 3704,3728 ---- bump = size_binop (MULT_EXPR, vs_minus_1, TYPE_SIZE_UNIT (scalar_type)); ptr = bump_vector_ptr (dataref_ptr, NULL, gsi, stmt, bump); ! new_stmt = gimple_build_assign_with_ops (BIT_AND_EXPR, ! NULL_TREE, ptr, ! build_int_cst (TREE_TYPE (ptr), ! -(HOST_WIDE_INT)TYPE_ALIGN_UNIT ((vectype)))); ! ptr = make_ssa_name (SSA_NAME_VAR (dataref_ptr), new_stmt); ! gimple_assign_set_lhs (new_stmt, ptr); ! vect_finish_stmt_generation (stmt, new_stmt, gsi); ! data_ref = build_simple_mem_ref (ptr); break; } case dr_explicit_realign_optimized: ! new_stmt = gimple_build_assign_with_ops (BIT_AND_EXPR, ! NULL_TREE, dataref_ptr, ! build_int_cst (TREE_TYPE (dataref_ptr), ! -(HOST_WIDE_INT)TYPE_ALIGN_UNIT ((vectype)))); ! new_temp = make_ssa_name (SSA_NAME_VAR (dataref_ptr), new_stmt); ! gimple_assign_set_lhs (new_stmt, new_temp); ! vect_finish_stmt_generation (stmt, new_stmt, gsi); ! data_ref = build_simple_mem_ref (new_temp); break; default: gcc_unreachable ();
On 06/29/2010 01:59 PM, Richard Guenther wrote: > Btw, the following hack at least passes the C vectorizer testsuite on > powerpc64 (and no rliwnm instructions are generated). So I plan to > continue down this road to eliminate ALIGN_INDIRECT_REF. Cool, it looks sane from the expander point of view. Paolo
Richard Guenther <rguenther@suse.de> wrote on 29/06/2010 12:03:58 PM: > From: Richard Guenther <rguenther@suse.de> > To: Paolo Bonzini <bonzini@gnu.org> > Cc: gcc-patches@gcc.gnu.org, Ira Rosen/Haifa/IBM@IBMIL > Date: 29/06/2010 12:05 PM > Subject: Re: [PATCH] mem-ref2 merge, core patch > > On Mon, 28 Jun 2010, Paolo Bonzini wrote: > > > On 06/28/2010 05:53 PM, Richard Guenther wrote: > > > On Mon, 28 Jun 2010, Paolo Bonzini wrote: > > > > > > > On 06/28/2010 02:38 PM, Richard Guenther wrote: > > > > > ALIGN_INDIRECT_REF I will replace with > > > > > > > > > > ptr = ptr& mask; > > > > > MEM_REF [ptr]; > > > > > > > > > > thus simply do what expansion replaces it with. > > > > > > > > So you are relying on TER to put together the pieces, right > (when it makes > > > > sense)? > > > > > > Well, I'm just expanding it on trees early. In expr.c we have > > > > > > case MISALIGNED_INDIRECT_REF: > > > case ALIGN_INDIRECT_REF: > > > case INDIRECT_REF: > > > { > > > ... > > > if (code == ALIGN_INDIRECT_REF) > > > { > > > int align = TYPE_ALIGN_UNIT (type); > > > op0 = gen_rtx_AND (address_mode, op0, GEN_INT (-align)); > > > op0 = memory_address_addr_space (mode, op0, as); > > > } > > > > > > we seem to rely on combine or whatever to do fancy things > > > > No, the AND is getting into the expansion directly if it is a legitimate > > address. > > Which is a hack, right? With the plan I have in mind we'd have > > ptr &= 3; // align the pointer > .. = *p; // aligned load > > which should expand fine (to an AND and to an aligned load). Then > combine can combine those two insns just fine to the aligning load. > > > > (I tried > > > to figure out where exactly things should be optimized but I even > > > failed to see in which cases exactly we end up generating > > > ALIGN_INDIRECT_REF in the first place ...) > > > > I think PPC vectorization uses it. > > Of course - that much I figured out by simple grepping ;) I just > didn't figure out why people thought it was a great idea to > invent ALIGN_INDIRECT_REF when a simple BIT_AND_EXPR on the tree-level > would do (not?). > > Thus it's now my part to find out the hard way ... but maybe > Ira has an idea? Sorry, I don't know. Here is a link to the relevant patch, but it doesn't help much http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01522.html. Ira > > Thanks, > Richard.
Index: gcc/dwarf2out.c =================================================================== --- gcc/dwarf2out.c (.../trunk) (revision 161367) +++ gcc/dwarf2out.c (.../branches/mem-ref2) (revision 161369) @@ -15159,6 +15159,11 @@ loc_list_from_tree (tree loc, int want_a } break; + case MEM_REF: + /* ??? FIXME. */ + if (!integer_zerop (TREE_OPERAND (loc, 1))) + return 0; + /* Fallthru. */ I have no idea what to do here (no clue about dwarf). Similar for the expand_debug_expr case.