Re: Fold BIT_FIELD_REF of a reference

[Marc Glisse <marc dot glisse at inria dot fr>]

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