Re: [PATCH] Add LANG_HOOKS_EMPTY_RECORD_P for C++ empty class

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Mon, Nov 23, 2015 at 10:00 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Mon, Nov 23, 2015 at 9:53 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Mon, Nov 23, 2015 at 7:22 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
>>> On Mon, Nov 23, 2015 at 3:53 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Mon, Nov 23, 2015 at 1:57 AM, Richard Biener
>>>> <richard.guenther@gmail.com> wrote:
>>>>> On Sat, Nov 21, 2015 at 12:46 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>> On Fri, Nov 20, 2015 at 2:17 PM, Jason Merrill <jason@redhat.com> wrote:
>>>>>>> On 11/20/2015 01:52 PM, H.J. Lu wrote:
>>>>>>>>
>>>>>>>> On Tue, Nov 17, 2015 at 4:22 AM, Richard Biener
>>>>>>>> <richard.guenther@gmail.com> wrote:
>>>>>>>>>
>>>>>>>>> On Tue, Nov 17, 2015 at 12:01 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>>>>
>>>>>>>>>> Empty record should be returned and passed the same way in C and C++.
>>>>>>>>>> This patch adds LANG_HOOKS_EMPTY_RECORD_P for C++ empty class, which
>>>>>>>>>> defaults to return false.  For C++, LANG_HOOKS_EMPTY_RECORD_P is defined
>>>>>>>>>> to is_really_empty_class, which returns true for C++ empty classes.  For
>>>>>>>>>> LTO, we stream out a bit to indicate if a record is empty and we store
>>>>>>>>>> it in TYPE_LANG_FLAG_0 when streaming in.  get_ref_base_and_extent is
>>>>>>>>>> changed to set bitsize to 0 for empty records.  Middle-end and x86
>>>>>>>>>> backend are updated to ignore empty records for parameter passing and
>>>>>>>>>> function value return.  Other targets may need similar changes.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Please avoid a new langhook for this and instead claim a bit in
>>>>>>>>> tree_type_common
>>>>>>>>> like for example restrict_flag (double-check it is unused for
>>>>>>>>> non-pointers).
>>>>>>>>
>>>>>>>>
>>>>>>>> There is no bit in tree_type_common I can overload.  restrict_flag is
>>>>>>>> checked for non-pointers to issue an error when it is used on
>>>>>>>> non-pointers:
>>>>>>>>
>>>>>>>>
>>>>>>>> /export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/template/qualttp20.C:19:38:
>>>>>>>> error: â__restrict__â qualifiers cannot be applied to âAS::Lâ
>>>>>>>>     typedef typename T::L __restrict__ r;// { dg-error "'__restrict__'
>>>>>>>> qualifiers cannot" "" }
>>>>>>>
>>>>>>>
>>>>>>> The C++ front end only needs to check TYPE_RESTRICT for this purpose on
>>>>>>> front-end-specific type codes like TEMPLATE_TYPE_PARM; cp_type_quals could
>>>>>>> handle that specifically if you change TYPE_RESTRICT to only apply to
>>>>>>> pointers.
>>>>>>>
>>>>>>
>>>>>> restrict_flag is also checked in this case:
>>>>>>
>>>>>> [hjl@gnu-6 gcc]$ cat x.i
>>>>>> struct dummy { };
>>>>>>
>>>>>> struct dummy
>>>>>> foo (struct dummy __restrict__ i)
>>>>>> {
>>>>>>   return i;
>>>>>> }
>>>>>> [hjl@gnu-6 gcc]$ gcc -S x.i -Wall
>>>>>> x.i:4:13: error: invalid use of ârestrictâ
>>>>>>  foo (struct dummy __restrict__ i)
>>>>>>              ^
>>>>>> x.i:4:13: error: invalid use of ârestrictâ
>>>>>> [hjl@gnu-6 gcc]$
>>>>>>
>>>>>> restrict_flag can't also be used to indicate `i' is an empty record.
>>>>>
>>>>> I'm sure this error can be done during parsing w/o relying on TYPE_RESTRICT.
>>>>>
>>>>> But well, use any other free bit (but do not enlarge
>>>>> tree_type_common).  Eventually
>>>>> you can free up a bit by putting sth into type_lang_specific currently
>>>>> using bits
>>>>> in tree_type_common.
>>>>
>>>> There are no bits in tree_type_common I can move.  Instead,
>>>> this patch overloads side_effects_flag in tree_base.  Tested on
>>>> Linux/x86-64.  OK for trunk?
>>>>
>>>
>>> Hi,
>>>
>>> Coincidentally a few months ago I was experimenting with making
>>> empty-struct function arguments zero-cost (and thus making them behave
>>> the same way as in GNU C).  My approach (patch attached) was to assign
>>> empty-struct arguments to a virtual register (instead of on the stack
>>> or to a hard register) during RTL call expansion.  These
>>> virtual-register assignments would then be trivially DCE'd later.
>>> This approach seemed to work surprisingly well with minimal code
>>> changes.  I wonder what
>>> your thoughts are on this approach..
>>
>> I don't think it works for C++ class.  empty_record_or_union_type_p
>> missed:
>>
>>     for (binfo = TYPE_BINFO (type), i = 0;
>>            BINFO_BASE_ITERATE (binfo, i, base_binfo); ++i)
>>         if (!is_really_empty_class (BINFO_TYPE (base_binfo)))
>>           return false;
>
> This above should not be needed as TYPE_FIELDS should include one
> already.  Or do you have prove it does not?

You can remove the above from

---
/* Returns true if TYPE contains no actual data, just various
   possible combinations of empty classes and possibly a vptr.  */

bool
is_really_empty_class (tree type)
{
  if (CLASS_TYPE_P (type))
    {
      tree field;
      tree binfo;
      tree base_binfo;
      int i;

      /* CLASSTYPE_EMPTY_P isn't set properly until the class is actually laid
         out, but we'd like to be able to check this before then.  */
      if (COMPLETE_TYPE_P (type) && is_empty_class (type))
        return true;

      for (binfo = TYPE_BINFO (type), i = 0;
           BINFO_BASE_ITERATE (binfo, i, base_binfo); ++i)
        if (!is_really_empty_class (BINFO_TYPE (base_binfo)))
          return false;
      for (field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
        if (TREE_CODE (field) == FIELD_DECL
            && !DECL_ARTIFICIAL (field)
            && !is_really_empty_class (TREE_TYPE (field)))
          return false;
      return true;
    }
  else if (TREE_CODE (type) == ARRAY_TYPE)
    return is_really_empty_class (TREE_TYPE (type));
  return false;
}
---

and see what happens.

-- 
H.J.