Re: [PATCH ARM]: PR67745: Fix function alignment after __attribute__ 2/2

[Christian Bruel <christian.bruel@st.com>]

[-- Attachment #1: Type: text/plain, Size: 1784 bytes --]



On 10/07/2015 07:37 PM, Bernd Schmidt wrote:
> On 10/07/2015 12:45 PM, Christian Bruel wrote:
>>
>>
>> On 10/07/2015 12:18 PM, Bernd Schmidt wrote:
>>> On 10/07/2015 09:04 AM, Christian Bruel wrote:
>>>> +      /* Similarly, relayout function's alignment if not forced.  */
>>>> +      if (!DECL_USER_ALIGN (fndecl)
>>>> +          && (TREE_CODE (fntype) != METHOD_TYPE
>>>> +          || TARGET_PTRMEMFUNC_VBIT_LOCATION !=
>>>> ptrmemfunc_vbit_in_pfn))
>>>> +        DECL_ALIGN (fndecl) = FUNCTION_BOUNDARY;
>>>>         }
>>>
>>> That's a very odd-looking condition. Why the vbit location test?
>>
>> This is for member functions to make sure that the lsb address is
>> reserved for the virtual function bit.
>>
>> see cp/decl.c:
>>
>>     /* If pointers to member functions use the least significant bit to
>>        indicate whether a function is virtual, ensure a pointer
>>        to this function will have that bit clear.  */
>>     if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn
>>         && TREE_CODE (type) == METHOD_TYPE
>>         && DECL_ALIGN (decl) < 2 * BITS_PER_UNIT)
>>       DECL_ALIGN (decl) = 2 * BITS_PER_UNIT;
>>
>> This happens for instance on i386 that aligns member functions on 2
>> bytes, bigger than the one byte required boundary. So we cannot
>> re-layout bellow that.
>
> Hmm, at least that would need to be spelled out in a comment, but I
> think this would better be abstracted as
>
> #define MINIMUM_FUNCTION_BOUNDARY(FN) \
>    ....
> in a header file, and used in both places.
>

OK, Similar pattern occurs at many other places, that changed also in 
the attached proposal.
Not fully tested (in particular the java part) and no ChangeLog. Just to 
make sure that we agree on the interface first.

Thanks

Christian

>
> Bernd
>

[-- Attachment #2: align5.patch --]
[-- Type: text/x-patch, Size: 3010 bytes --]

Index: gcc/builtins.c
===================================================================
304a305,306
>       tree fntype = exp ? TREE_TYPE (exp) : NULL_TREE;
> 
309,310c311,312
<       if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn)
< 	align = 2 * BITS_PER_UNIT;
---
>       align = MAX (minimum_function_boundary (exp), align);
> 
Index: gcc/cp/decl.c
===================================================================
7855,7858c7855
<   if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn
<       && TREE_CODE (type) == METHOD_TYPE
<       && DECL_ALIGN (decl) < 2 * BITS_PER_UNIT)
<     DECL_ALIGN (decl) = 2 * BITS_PER_UNIT;
---
>   DECL_ALIGN (decl) = MAX (minimum_function_boundary (decl), DECL_ALIGN (decl));
Index: gcc/cp/lambda.c
===================================================================
1010,1012c1010,1011
<   if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn
<       && DECL_ALIGN (fn) < 2 * BITS_PER_UNIT)
<     DECL_ALIGN (fn) = 2 * BITS_PER_UNIT;
---
>   extern int need_vptr_align (void);
>   DECL_ALIGN (fn) = MAX (minimum_function_boundary (fn), DECL_ALIGN (fn));
1045,1047c1044
<   if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn
<       && DECL_ALIGN (fn) < 2 * BITS_PER_UNIT)
<     DECL_ALIGN (fn) = 2 * BITS_PER_UNIT;
---
>   DECL_ALIGN (fn) = MAX (minimum_function_boundary (fn), DECL_ALIGN (fn));
Index: gcc/cp/method.c
===================================================================
1852c1852
<   
---
> 
1856,1858c1856
<   if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn
<       && DECL_ALIGN (fn) < 2 * BITS_PER_UNIT)
<     DECL_ALIGN (fn) = 2 * BITS_PER_UNIT;
---
>   DECL_ALIGN (fn) = MAX (minimum_function_boundary (fn), DECL_ALIGN (fn));
Index: gcc/function.c
===================================================================
4791a4792,4806
> unsigned int
> minimum_function_boundary (tree fndecl)
> {
>   gcc_assert (fndecl);
> 
>   tree fntype = TREE_TYPE (fndecl);
> 
>   if (TREE_CODE (fntype) == METHOD_TYPE
>       && TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn
>       && FUNCTION_BOUNDARY < 2 * BITS_PER_UNIT)
>       return 2 * BITS_PER_UNIT;
> 
>   return FUNCTION_BOUNDARY;
> }
> 
4842a4858,4861
> 
> 	  /* Similarly, relayout function's alignment if not forced.  */
> 	  if (!DECL_USER_ALIGN (fndecl))
> 	    DECL_ALIGN (fndecl) = minimum_function_boundary (fndecl);
Index: gcc/function.h
===================================================================
601c601
< 
---
> extern unsigned int minimum_function_boundary (tree);
Index: gcc/java/class.c
===================================================================
785,788c785,786
<   if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn
<       && !(access_flags & ACC_STATIC)
<       && DECL_ALIGN (fndecl) < 2 * BITS_PER_UNIT)
<     DECL_ALIGN (fndecl) = 2 * BITS_PER_UNIT;
---
>   if !(access_flags & ACC_STATIC)
>     DECL_ALIGN (fndecl) = MAX (minimum_function_boundary (decl), DECL_ALIGN (fndecl));