This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fold BIT_FIELD_REF of a reference


On Thu, Apr 4, 2013 at 10:53 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> 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?

Sure.  A vector not in memory is 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.

I'd like to see a case where this happens.

>>> 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.

No, it returns 'exp' which is still MEM_REF[&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.

Yeah, I definitely know that frontends may not expect MEM_REFs at all
(so folding to MEM_REF from fold-const.c is probably not the very best idea).
For that issue, simply do the transform from gimple_fold only.

Richard.

> --
> Marc Glisse


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]