Fix PR80295[aarch64] [7/8 Regression] ICE in __builtin_update_setjmp_buf expander

Hi,

This patch fixes the aarch64 bug 80295
https://gcc.gnu.org/PR80295

The aarch64 backend has multiple places that miss the handling of TARGET_ILP32.
in the patch, we added correct handling of TARGET_ILP32 into aarch64 backend. 

a new small testing case is added.

bootstrapped and tested on aarch64-unknown-linux-gnu with no regression.

thanks.

Qing

On Sep 25 2017, Qing Zhao <qing.zhao@oracle.com> wrote:

> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 6c3ef76..876e9e3 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -3693,7 +3693,9 @@ aarch64_expand_prologue (void)
>  				       stack_pointer_rtx,
>  				       GEN_INT (callee_offset)));
>        RTX_FRAME_RELATED_P (insn) = 1;
> -      emit_insn (gen_stack_tie (stack_pointer_rtx, hard_frame_pointer_rtx));
> +      emit_insn (TARGET_ILP32 ? 
> +                 gen_stack_tiesi (stack_pointer_rtx, hard_frame_pointer_rtx) :
> +                 gen_stack_tiedi (stack_pointer_rtx, hard_frame_pointer_rtx));

GNU style is line break before the operator, not after.

Andreas.

Hi, Andreas,

thanks for the comment. 

> GNU style is line break before the operator, not after.

updated per your comment.

Qing.

---
 gcc/config/aarch64/aarch64.c               | 12 +++++++++---
 gcc/config/aarch64/aarch64.h               |  2 +-
 gcc/config/aarch64/aarch64.md              |  6 +++---
 gcc/testsuite/gcc.target/aarch64/pr80295.c |  8 ++++++++
 4 files changed, 21 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/pr80295.c

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 6c3ef76..ff0890d 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -3693,7 +3693,9 @@ aarch64_expand_prologue (void)
 				       stack_pointer_rtx,
 				       GEN_INT (callee_offset)));
       RTX_FRAME_RELATED_P (insn) = 1;
-      emit_insn (gen_stack_tie (stack_pointer_rtx, hard_frame_pointer_rtx));
+      emit_insn (TARGET_ILP32 
+                 ? gen_stack_tiesi (stack_pointer_rtx, hard_frame_pointer_rtx)
+                 : gen_stack_tiedi (stack_pointer_rtx, hard_frame_pointer_rtx));
     }
 
   aarch64_save_callee_saves (DImode, callee_offset, R0_REGNUM, R30_REGNUM,
@@ -3750,7 +3752,9 @@ aarch64_expand_epilogue (bool for_sibcall)
   if (final_adjust > crtl->outgoing_args_size || cfun->calls_alloca
       || crtl->calls_eh_return)
     {
-      emit_insn (gen_stack_tie (stack_pointer_rtx, stack_pointer_rtx));
+      emit_insn (TARGET_ILP32 
+                 ? gen_stack_tiesi (stack_pointer_rtx, stack_pointer_rtx)
+                 : gen_stack_tiedi (stack_pointer_rtx, stack_pointer_rtx));
       need_barrier_p = false;
     }
 
@@ -3774,7 +3778,9 @@ aarch64_expand_epilogue (bool for_sibcall)
 				callee_adjust != 0, &cfi_ops);
 
   if (need_barrier_p)
-    emit_insn (gen_stack_tie (stack_pointer_rtx, stack_pointer_rtx));
+    emit_insn (TARGET_ILP32 
+               ? gen_stack_tiesi (stack_pointer_rtx, stack_pointer_rtx)
+               : gen_stack_tiedi (stack_pointer_rtx, stack_pointer_rtx));
 
   if (callee_adjust != 0)
     aarch64_pop_regs (reg1, reg2, callee_adjust, &cfi_ops);
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 8fada9e..df58442 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -782,7 +782,7 @@ typedef struct
 /* Specify the machine mode that the hardware addresses have.
    After generation of rtl, the compiler makes no further distinction
    between pointers and any other objects of this machine mode.  */
-#define Pmode		DImode
+#define Pmode		(TARGET_ILP32 ? SImode : DImode)
 
 /* A C expression whose value is zero if pointers that need to be extended
    from being `POINTER_SIZE' bits wide to `Pmode' are sign-extended and
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index bb7f2c0..30853b2 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -5533,10 +5533,10 @@
   [(set_attr "type" "call")
    (set_attr "length" "16")])
 
-(define_insn "stack_tie"
+(define_insn "stack_tie<GPI:mode>"
   [(set (mem:BLK (scratch))
-	(unspec:BLK [(match_operand:DI 0 "register_operand" "rk")
-		     (match_operand:DI 1 "register_operand" "rk")]
+	(unspec:BLK [(match_operand:GPI 0 "register_operand" "rk")
+		     (match_operand:GPI 1 "register_operand" "rk")]
 		    UNSPEC_PRLG_STK))]
   ""
   ""
diff --git a/gcc/testsuite/gcc.target/aarch64/pr80295.c b/gcc/testsuite/gcc.target/aarch64/pr80295.c
new file mode 100644
index 0000000..b3866d8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr80295.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-mabi=ilp32" } */
+
+void f (void *b) 
+{ 
+  __builtin_update_setjmp_buf (b); 
+}
+

On 25/09/17 17:35, Qing Zhao wrote:
> Hi, Andreas,
> 
> thanks for the comment. 
> 
>> GNU style is line break before the operator, not after.
> 
> updated per your comment.
> 
> Qing.
> 
> ---
>  gcc/config/aarch64/aarch64.c               | 12 +++++++++---
>  gcc/config/aarch64/aarch64.h               |  2 +-
>  gcc/config/aarch64/aarch64.md              |  6 +++---
>  gcc/testsuite/gcc.target/aarch64/pr80295.c |  8 ++++++++
>  4 files changed, 21 insertions(+), 7 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/pr80295.c
> 
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 6c3ef76..ff0890d 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -3693,7 +3693,9 @@ aarch64_expand_prologue (void)
>  				       stack_pointer_rtx,
>  				       GEN_INT (callee_offset)));
>        RTX_FRAME_RELATED_P (insn) = 1;
> -      emit_insn (gen_stack_tie (stack_pointer_rtx, hard_frame_pointer_rtx));
> +      emit_insn (TARGET_ILP32 
> +                 ? gen_stack_tiesi (stack_pointer_rtx, hard_frame_pointer_rtx)
> +                 : gen_stack_tiedi (stack_pointer_rtx, hard_frame_pointer_rtx));
>      }
>  
>    aarch64_save_callee_saves (DImode, callee_offset, R0_REGNUM, R30_REGNUM,
> @@ -3750,7 +3752,9 @@ aarch64_expand_epilogue (bool for_sibcall)
>    if (final_adjust > crtl->outgoing_args_size || cfun->calls_alloca
>        || crtl->calls_eh_return)
>      {
> -      emit_insn (gen_stack_tie (stack_pointer_rtx, stack_pointer_rtx));
> +      emit_insn (TARGET_ILP32 
> +                 ? gen_stack_tiesi (stack_pointer_rtx, stack_pointer_rtx)
> +                 : gen_stack_tiedi (stack_pointer_rtx, stack_pointer_rtx));
>        need_barrier_p = false;
>      }
>  
> @@ -3774,7 +3778,9 @@ aarch64_expand_epilogue (bool for_sibcall)
>  				callee_adjust != 0, &cfi_ops);
>  
>    if (need_barrier_p)
> -    emit_insn (gen_stack_tie (stack_pointer_rtx, stack_pointer_rtx));
> +    emit_insn (TARGET_ILP32 
> +               ? gen_stack_tiesi (stack_pointer_rtx, stack_pointer_rtx)
> +               : gen_stack_tiedi (stack_pointer_rtx, stack_pointer_rtx));
>  
>    if (callee_adjust != 0)
>      aarch64_pop_regs (reg1, reg2, callee_adjust, &cfi_ops);
> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
> index 8fada9e..df58442 100644
> --- a/gcc/config/aarch64/aarch64.h
> +++ b/gcc/config/aarch64/aarch64.h
> @@ -782,7 +782,7 @@ typedef struct
>  /* Specify the machine mode that the hardware addresses have.
>     After generation of rtl, the compiler makes no further distinction
>     between pointers and any other objects of this machine mode.  */
> -#define Pmode		DImode
> +#define Pmode		(TARGET_ILP32 ? SImode : DImode)

This is wrong.  AArch64 has both ptr_mode and Pmode.  Pmode must always
be DImode as (internally) all addresses must be 64-bit.  ptr_mode
reflects the ABI choice of 32/64-bit language level addresses.  The
register SP must always be a 64-bit value, even when all the top 32 bits
are zero.

R.

>  
>  /* A C expression whose value is zero if pointers that need to be extended
>     from being `POINTER_SIZE' bits wide to `Pmode' are sign-extended and
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index bb7f2c0..30853b2 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -5533,10 +5533,10 @@
>    [(set_attr "type" "call")
>     (set_attr "length" "16")])
>  
> -(define_insn "stack_tie"
> +(define_insn "stack_tie<GPI:mode>"
>    [(set (mem:BLK (scratch))
> -	(unspec:BLK [(match_operand:DI 0 "register_operand" "rk")
> -		     (match_operand:DI 1 "register_operand" "rk")]
> +	(unspec:BLK [(match_operand:GPI 0 "register_operand" "rk")
> +		     (match_operand:GPI 1 "register_operand" "rk")]
>  		    UNSPEC_PRLG_STK))]
>    ""
>    ""
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr80295.c b/gcc/testsuite/gcc.target/aarch64/pr80295.c
> new file mode 100644
> index 0000000..b3866d8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr80295.c
> @@ -0,0 +1,8 @@
> +/* { dg-do compile } */
> +/* { dg-options "-mabi=ilp32" } */
> +
> +void f (void *b) 
> +{ 
> +  __builtin_update_setjmp_buf (b); 
> +}
> +
>

Richard,

thanks for your review and comments.

> On Oct 5, 2017, at 11:50 AM, Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote:
> 
> On 25/09/17 17:35, Qing Zhao wrote:
>> 
>> --- a/gcc/config/aarch64/aarch64.h
>> +++ b/gcc/config/aarch64/aarch64.h
>> @@ -782,7 +782,7 @@ typedef struct
>> /* Specify the machine mode that the hardware addresses have.
>>    After generation of rtl, the compiler makes no further distinction
>>    between pointers and any other objects of this machine mode.  */
>> -#define Pmode		DImode
>> +#define Pmode		(TARGET_ILP32 ? SImode : DImode)
> 
> This is wrong.  AArch64 has both ptr_mode and Pmode.  Pmode must always
> be DImode as (internally) all addresses must be 64-bit.  ptr_mode
> reflects the ABI choice of 32/64-bit language level addresses.  The
> register SP must always be a 64-bit value, even when all the top 32 bits
> are zero.


So,  in ilp32 ABI, compared to LP64,  the only difference is:

       long and pointer size are different

all the others are the same, including:

    registers size are 64 bits for both ilp32 and LP64.
    stack poping and pushing by 64 bits for both ilp32 and LP64.

is the above right?

what’s the other difference between ilp32 and LP64?

thanks a lot for your help.

Qing