[RFC][PATCH][X86_64] Eliminate PLT stubs for specified external functions via -fno-plt=

[Sriraman Tallam]

Hi HJ,

Is this ok to commit?

Thanks
Sri

On Thu, May 28, 2015 at 11:03 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> On Thu, May 28, 2015 at 5:05 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Thu, May 28, 2015 at 4:54 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>>> On Thu, May 28, 2015 at 2:52 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Thu, May 28, 2015 at 2:27 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>>>>> On Thu, May 28, 2015 at 2:01 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>> On Thu, May 28, 2015 at 1:54 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>>>>>>> On Thu, May 28, 2015 at 12:05 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>> On Thu, May 28, 2015 at 11:50 AM, Sriraman Tallam <tmsriram@google.com> wrote:
>>>>>>>>> On Thu, May 28, 2015 at 11:42 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>>>> On Thu, May 28, 2015 at 11:34 AM, Sriraman Tallam <tmsriram@google.com> wrote:
>>>>>>>>>>> I have attached a patch that adds the new attribute "noplt".  Please review.
>>>>>>>>>>>
>>>>>>>>>>> * config/i386/i386.c (avoid_plt_to_call): New function.
>>>>>>>>>>> (ix86_output_call_insn): Generate indirect call for functions
>>>>>>>>>>> marked with "noplt" attribute.
>>>>>>>>>>> (attribute_spec ix86_attribute_): Define new attribute "noplt".
>>>>>>>>>>> * doc/extend.texi: Document new attribute "noplt".
>>>>>>>>>>> * gcc.target/i386/noplt-1.c: New testcase.
>>>>>>>>>>> * gcc.target/i386/noplt-2.c: New testcase.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 2 comments:
>>>>>>>>>>
>>>>>>>>>> 1. Don't remove "%!" prefix before call/jmp.  It is needed for MPX.
>>>>>>>>>> 2. Don't you need to check
>>>>>>>>>>
>>>>>>>>>>       && !TARGET_MACHO
>>>>>>>>>>       && !TARGET_SEH
>>>>>>>>>>       && !TARGET_PECOFF
>>>>>>>>>>
>>>>>>>>>> since it only works for ELF.
>>>>>>>>>
>>>>>>>>> Ok, I will make this change. OTOH, is it just better to piggy-back on
>>>>>>>>> existing -fno-plt change by Alex in calls.c
>>>>>>>>> and do this:
>>>>>>>>>
>>>>>>>>> Index: calls.c
>>>>>>>>> ===================================================================
>>>>>>>>> --- calls.c (revision 223720)
>>>>>>>>> +++ calls.c (working copy)
>>>>>>>>> @@ -226,9 +226,11 @@ prepare_call_address (tree fndecl_or_type, rtx fun
>>>>>>>>>         && targetm.small_register_classes_for_mode_p (FUNCTION_MODE))
>>>>>>>>>        ? force_not_mem (memory_address (FUNCTION_MODE, funexp))
>>>>>>>>>        : memory_address (FUNCTION_MODE, funexp));
>>>>>>>>> -  else if (flag_pic && !flag_plt && fndecl_or_type
>>>>>>>>> +  else if (fndecl_or_type
>>>>>>>>>     && TREE_CODE (fndecl_or_type) == FUNCTION_DECL
>>>>>>>>> -   && !targetm.binds_local_p (fndecl_or_type))
>>>>>>>>> +   && !targetm.binds_local_p (fndecl_or_type)
>>>>>>>>> +   && ((flag_pic && !flag_plt)
>>>>>>>>> +       || (lookup_attribute ("noplt", DECL_ATTRIBUTES(fndecl_or_type)))))
>>>>>>>>>      {
>>>>>>>>>        funexp = force_reg (Pmode, funexp);
>>>>>>>>>      }
>>>>>>>>>
>>>>>>>>
>>>>>>>> Does it work on non-PIC calls?
>>>>>>>
>>>>>>> You are right, it doesnt work.  I have attached the patch with the
>>>>>>> changes you mentioned.
>>>>>>>
>>>>>>
>>>>>> Since direct_p is true, do wee need
>>>>>>
>>>>>> +  if (GET_CODE (call_op) != SYMBOL_REF
>>>>>> +      || SYMBOL_REF_LOCAL_P (call_op))
>>>>>> +    return false;
>>>>>
>>>>> We do need it right because  for this case below, I do not want an
>>>>> indirect call:
>>>>>
>>>>> __attribute__((noplt))
>>>>> int foo() {
>>>>>   return 0;
>>>>> }
>>>>>
>>>>> int main()
>>>>> {
>>>>>   return foo();
>>>>> }
>>>>>
>>>>> Assuming foo is not inlined, if I remove the lines you mentioned, I
>>>>> will get an indirect call which is unnecessary.
>>>>>
>>>>
>>>> I meant the "GET_CODE (call_op) != SYMBOL_REF" part isn't
>>>> needed.
>>>
>>> I should have realized that :), sorry.  Patch fixed.
>>>
>>
>> --- testsuite/gcc.target/i386/noplt-1.c (revision 0)
>> +++ testsuite/gcc.target/i386/noplt-1.c (working copy)
>> @@ -0,0 +1,13 @@
>> +/* { dg-do compile { target x86_64-*-* } } */
>> ...
>> +/* { dg-final { scan-assembler "call\[
>> \t\]\\*.*foo.*@GOTPCREL\\(%rip\\)" } } */
>>
>> The test will fail on Windows and Darwin.
>
> Changed to use x86_64-*-linux* target.
>
>>
>>
>> --
>> H.J.