On Thu, 4 Apr 2013, Richard Biener wrote:

+      if ((handled_component_p (arg0) || TREE_CODE (arg0) == MEM_REF)


This check means the optimization is not performed for
BIT_FIELD_REF[a, *, CST] which I see no particularly good reason for.


Er, are you trying to get rid of all BIT_FIELD_REFs? Why would you want to
replace them with a MEM_REF? I actually think my patch already replaces too
many.

Yes, when I filed the bug I was working on bitfield lowering and the only
BIT_FIELD_REFs that would survive would be bitfield extracts from
registers.

Can't a vector (not in memory) count as a register?

Thus, BIT_FIELD_REFs on memory would be lowered as

 reg_2 = MEM[ ... ];
 res_3 = BIT_FIELD_REF [reg_2, ...];

with an appropriately aligned / bigger size memory MEM.

As a first step I wanted to lower all BIT_FIELD_REFs that can be expressed
directly as memory access (byte-aligned and byte-size) to regular memory
accesses.

But the transformation on BIT_FIELD_REF[A,...] will take the address of A even if A is not something that is ok with having its address taken.

By the way, if I understand the code correctly,
get_addr_base_and_unit_offset can just as easily return an object or an
address, when it is called on a MEM_REF. That may be an issue as well.

It should always return an object.

I am probably missing something. Looking in tree-flow-inline.c, for MEM_REF[a,...]:

       case MEM_REF:
         {
           tree base = TREE_OPERAND (exp, 0);
           if (valueize
               && TREE_CODE (base) == SSA_NAME)
             base = (*valueize) (base);

valueize is 0.

           /* Hand back the decl for MEM[&decl, off].  */
           if (TREE_CODE (base) == ADDR_EXPR)

not the case here.

             {
               if (!integer_zerop (TREE_OPERAND (exp, 1)))
                 {
                   double_int off = mem_ref_offset (exp);
                   gcc_assert (off.high == -1 || off.high == 0);
                   byte_offset += off.to_shwi ();
                 }
               exp = TREE_OPERAND (base, 0);
             }
           goto done;

it returns a, which afaiu is an address.
For MEM_REF[&b] it does return b.

Maybe I should just forget about this patch for now...

If it breaks all over the testsuite if generalized then yes (is it just
dump scans that fail or are you seeing "real" issues?)

Verification failure for ADDR_EXPR: is_gimple_address (t)

unexpected expression 'v' of kind mem_ref in cxx_eval_constant_expression

The first one in particular affects almost all vector tests, so it might hide other issues.

--
Marc Glisse

Reply via email to