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

[Jiong Wang <jiong.wang@arm.com>]

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


James Greenhalgh writes:

> On Thu, Aug 06, 2015 at 05:16:33PM +0100, Jiong Wang wrote:
>
> 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


  Thanks for noticed this.

  As it's -fpic in dg-option, so they are supposed to work under -fpic,
  while I was checking the instruction sequences for -fPIC which is wrong.

  It's passed on my local machine because I was doing cross-check and
  there is no cross binutils installed, I only built cc1, then run the
  check. After double check I found actually all those scan-assemble
  test have not been triggered, because looks like the dejagnu was using
  local x86 assembler to do some prerequite check, then found -EL not
  supported, then those prerequite check returns false.

  Even worse, as -fpic for AArch64 will fall back to -fPIC if the
  installed aarch64 binutils don't support recently added relocation
  types for -fpic, so even I have wrote correct instruction sequences in
  this testcase, it will fail on those environment where old binutils
  installed, and... even new binutils installed, they may still fail if
  the user pre-configure gcc with -mabi=ilp32, as for ILP32, the
  relocation modifer for small code small for -fPIC is gotpage_14
  instead of gotpage_15 for LP64.

  Unfortunally, after all above considered, these testcases still fail
  if the user force -mcmodel=tiny to the compilation options, because
  those dejagnu target effective check will not know those extra options
  user added.

  After second think, I found my previous checking logic is not
  good. It's better we check the final branch type instead of be
  bothered by those relocation modifiers.

  Because the fundanmental changes -fno-plt bring us is it turn direct
  branch into indirect branch, then turn "bl/b" into "blr/br", while
  those GOT reloated modifers are just for preparing the branch
  destination register.

  Patch pass -fpic/-fPIC/binutils-without-fpic/binutils-with-fpic/ilp32.

  Should be OK now.

  Commited as obivious.

  Thanks.

  2015-08-07  Jiong Wang  <jiong.wang@arm.com>

  gcc/testsuite/
    * gcc.target/aarch64/noplt_1.c: Check branch type instead of
    relocation modifers.
    * gcc.target/aarch64/noplt_2.c: Likewise.
    * gcc.target/aarch64/noplt_3.c: Likewise.

-- 
Regards,
Jiong


[-- Attachment #2: fix.patch --]
[-- Type: text/x-diff, Size: 1737 bytes --]

diff --git a/gcc/testsuite/gcc.target/aarch64/noplt_1.c b/gcc/testsuite/gcc.target/aarch64/noplt_1.c
index 4e9bb62..731fcae 100644
--- a/gcc/testsuite/gcc.target/aarch64/noplt_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/noplt_1.c
@@ -11,5 +11,5 @@ foo (int a)
   return b[a];
 }
 
-/* { dg-final { scan-assembler "#:got:" { target { aarch64_tiny || aarch64_small } } } } */
-/* { dg-final { scan-assembler "#:got_lo12:" { target aarch64_small } } } */
+/* { dg-final { scan-assembler "blr" } } */
+/* { dg-final { scan-assembler-not "bl\t" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/noplt_2.c b/gcc/testsuite/gcc.target/aarch64/noplt_2.c
index 718999b..3be94aa 100644
--- a/gcc/testsuite/gcc.target/aarch64/noplt_2.c
+++ b/gcc/testsuite/gcc.target/aarch64/noplt_2.c
@@ -14,5 +14,5 @@ foo (int a)
   return b0[a] + b1[a];
 }
 
-/* { dg-final { scan-assembler-times "#:got:" 1 { target { aarch64_tiny || aarch64_small } } } } */
-/* { dg-final { scan-assembler-times "#:got_lo12:" 1 { target aarch64_small } } } */
+/* { dg-final { scan-assembler-times "blr" 1 } } */
+/* { dg-final { scan-assembler-times "bl\t" 1 } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/noplt_3.c b/gcc/testsuite/gcc.target/aarch64/noplt_3.c
index c1993b6..ef6e65d 100644
--- a/gcc/testsuite/gcc.target/aarch64/noplt_3.c
+++ b/gcc/testsuite/gcc.target/aarch64/noplt_3.c
@@ -16,5 +16,5 @@ 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 } } } */
+/* { dg-final { scan-assembler-times "br" 2 } } */
+/* { dg-final { scan-assembler-not "b\t" } } */