diff mbox

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

Message ID 56167745.1080409@st.com
State New
Headers show

Commit Message

Christian Bruel Oct. 8, 2015, 2:01 p.m. UTC 
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
>

Comments

Bernd Schmidt Oct. 8, 2015, 2:05 p.m. UTC | #1 
On 10/08/2015 04:01 PM, Christian Bruel wrote:

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

That looks like a plain diff rather than context or unified, which is 
very hard to read, but I think this is the approach I was looking for. 
Well done spotting the other places.


Bernd
diff mbox

Patch

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));