diff mbox

[COMMITTED,AArch64,sibcall] Tighten direct call pattern to repair -fno-plt

Message ID n99k2t8s84e.fsf@arm.com
State New
Headers show

Commit Message

Jiong Wang Aug. 6, 2015, 4:16 p.m. UTC
James Greenhalgh writes:

> On Tue, Jul 21, 2015 at 01:42:35PM +0100, Jiong Wang wrote:
>> 
>> Jiong Wang writes:
>> 
>> > Alexander Monakov writes:
>> >
>> >>> Attachment is the patch which repair -fno-plt support for AArch64.
>> >>> 
>> >>> aarch64_is_noplt_call_p will only be true if:
>> >>> 
>> >>>   * gcc is generating position independent code.
>> >>>   * function symbol has declaration.
>> >>>   * either -fno-plt or "(no_plt)" attribute specified.
>> >>>   * it's a external function.
>> >>>   
>> >>> OK for trunk?
>> >>> 
>> >>> 2015-07-16  Jiong Wang  <jiong.wang@arm.com>
>> >>> 
>> >>> gcc/
>> >>>   * config/aarch64/aarch64-protos.h (aarch64_is_noplt_call_p): New
>> >>>   declaration.
>> >>>   * config/aarch64/aarch64.c (aarch64_is_noplt_call_p): New function.
>> >>>   * config/aarch64/aarch64.md (call_value_symbol): Check noplt
>> >>>   scenarios.
>> >>>   (call_symbol): Ditto.
>> >>
>> >> Shouldn't the same treatment be applied to tailcall (sibcall_{,value_}symbol)
>> >> patterns?  I guess it could be done as a followup patch, but would be nice if
>> >> that isn't forgotten.
>> >
>> > Thanks for the remaind, that will be done as a followup patch.
>> 
>> Patch attached.
>> 
>> Added one more restriction to "Usf" constraint which is used by sibcall
>> pattern when matching direct call.
>> 
>> given example like
>> 
>> void
>> cal_novalue (int a)
>> {
>>   dec (a);
>> }
>> 
>> when -fpic -fno-plt specified we now generate:
>> 
>> cal:
>>         adrp    x1, :got:dec
>>         ldr     x1, [x1, #:got_lo12:dec]
>>         br      x1
>> 
>> instead of:
>> 
>> cal:
>>         b dec
>
> OK.
>
> Thanks,
> James
>

Committed the following patch which done minor adjustments so the
testcases can work well on any of tiny, small, large model.

Thanks.

Comments

James Greenhalgh Aug. 7, 2015, 8:22 a.m. UTC | #1
On Thu, Aug 06, 2015 at 05:16:33PM +0100, Jiong Wang wrote:
> 
> James Greenhalgh writes:
> 
> > On Tue, Jul 21, 2015 at 01:42:35PM +0100, Jiong Wang wrote:
> >> 
> >> Jiong Wang writes:
> >> 
> >> > Alexander Monakov writes:
> >> >
> >> >>> Attachment is the patch which repair -fno-plt support for AArch64.
> >> >>> 
> >> >>> aarch64_is_noplt_call_p will only be true if:
> >> >>> 
> >> >>>   * gcc is generating position independent code.
> >> >>>   * function symbol has declaration.
> >> >>>   * either -fno-plt or "(no_plt)" attribute specified.
> >> >>>   * it's a external function.
> >> >>>   
> >> >>> OK for trunk?
> >> >>> 
> >> >>> 2015-07-16  Jiong Wang  <jiong.wang@arm.com>
> >> >>> 
> >> >>> gcc/
> >> >>>   * config/aarch64/aarch64-protos.h (aarch64_is_noplt_call_p): New
> >> >>>   declaration.
> >> >>>   * config/aarch64/aarch64.c (aarch64_is_noplt_call_p): New function.
> >> >>>   * config/aarch64/aarch64.md (call_value_symbol): Check noplt
> >> >>>   scenarios.
> >> >>>   (call_symbol): Ditto.
> >> >>
> >> >> Shouldn't the same treatment be applied to tailcall (sibcall_{,value_}symbol)
> >> >> patterns?  I guess it could be done as a followup patch, but would be nice if
> >> >> that isn't forgotten.
> >> >
> >> > Thanks for the remaind, that will be done as a followup patch.

Hi Jiong,

The new testcases introduced in this and the related patch are failing
for me on aarch64-none-elf:

    aarch64-none-elf

	NA->FAIL: gcc.target/aarch64/noplt_1.c scan-assembler
	NA->FAIL: gcc.target/aarch64/noplt_2.c scan-assembler-times
	NA->FAIL: gcc.target/aarch64/noplt_3.c scan-assembler-times

For this invocation:
 
  .../build/obj/gcc2/gcc/xgcc -B.../build/obj/gcc2/gcc/ .../src/gcc/testsuite/gcc.target/aarch64/noplt_1.c -fno-diagnostics-show-caret -fdiagnostics-color=never -O2 -fno-plt -fpic -S  -mcmodel=small -o noplt_1.s

I get this code generation for the small memory model:

foo:
	stp	x29, x30, [sp, -32]!
	adrp	x1, _GLOBAL_OFFSET_TABLE_
	add	x29, sp, 0
	str	x19, [sp, 16]
	mov	w19, w0
	ldr	x0, [x1, #:gotpage_lo15:bar]
	blr	x0
	ldr	w0, [x0, w19, sxtw 2]
	ldr	x19, [sp, 16]
	ldp	x29, x30, [sp], 32
	ret
	.size	foo, .-foo

Which uses a different relocation.

Did you intend for these tests to be run with -fPIC -fno-plt rather than
-fpic -fno-plt, or does this indicate a bug?

Thanks,
James
diff mbox

Patch

Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 226681)
+++ gcc/ChangeLog	(working copy)
@@ -1,5 +1,10 @@ 
 2015-08-06  Jiong Wang  <jiong.wang@arm.com>
 
+	* config/aarch64/constraints.md (Usf): Add the test of
+	aarch64_is_noplt_call_p.
+
+2015-08-06  Jiong Wang  <jiong.wang@arm.com>
+
 	* config/aarch64/aarch64-protos.h (aarch64_is_noplt_call_p): New declaration.
 	* config/aarch64/aarch64.c (aarch64_is_noplt_call_p): New function.
 	* config/aarch64/aarch64.md (call_value_symbol): Check noplt scenarios.
Index: gcc/config/aarch64/constraints.md
===================================================================
--- gcc/config/aarch64/constraints.md	(revision 226680)
+++ gcc/config/aarch64/constraints.md	(working copy)
@@ -101,8 +101,9 @@ 
        (match_test "(unsigned HOST_WIDE_INT) ival < 64")))
 
 (define_constraint "Usf"
-  "@internal Usf is a symbol reference."
-  (match_code "symbol_ref"))
+  "@internal Usf is a symbol reference under the context where plt stub allowed."
+  (and (match_code "symbol_ref")
+       (match_test "!aarch64_is_noplt_call_p (op)")))
 
 (define_constraint "UsM"
   "@internal
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(revision 226681)
+++ gcc/testsuite/ChangeLog	(working copy)
@@ -1,5 +1,9 @@ 
 2015-08-06  Jiong Wang  <jiong.wang@arm.com>
 
+	* gcc.target/aarch64/noplt_3.c: New testcase.
+
+2015-08-06  Jiong Wang  <jiong.wang@arm.com>
+
 	* gcc.target/aarch64/noplt_1.c: New testcase.
 	* gcc.target/aarch64/noplt_2.c: Likewise.
 
Index: gcc/testsuite/gcc.target/aarch64/noplt_3.c
===================================================================
--- gcc/testsuite/gcc.target/aarch64/noplt_3.c	(revision 0)
+++ gcc/testsuite/gcc.target/aarch64/noplt_3.c	(working copy)
@@ -0,0 +1,20 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fpic -fno-plt" } */
+/* { dg-skip-if "-mcmodel=large, no support for -fpic" { aarch64-*-* }  { "-mcmodel=large" } { "" } } */
+
+int dec (int);
+
+int
+cal (int a)
+{
+  return dec (a);
+}
+
+void
+cal_novalue (int a)
+{
+  dec (a);
+}
+
+/* { dg-final { scan-assembler-times "#:got:" 2 { target { aarch64_tiny || aarch64_small } } } } */
+/* { dg-final { scan-assembler-times "#:got_lo12:" 2 { target aarch64_small } } } */