Re: [PATCH] PR target/96759 - Handle global variable assignment from misaligned structure/PARALLEL return values.

[Richard Biener <rguenther@suse.de>]

On Thu, 10 Sep 2020, Kito Cheng wrote:

> In g:70cdb21e579191fe9f0f1d45e328908e59c0179e, DECL/global variable has handled
> misaligned stores, but it didn't handle PARALLEL values, and I refer the
> other part of this function, I found the PARALLEL need handled by
> emit_group_* functions, so I add a check, and using emit_group_store if
> storing a PARALLEL value, also checked this change didn't break the
> testcase(gcc.target/arm/unaligned-argument-3.c) added by the orginal changes.
> 
> For riscv64 target, struct S {int a; double b;} will pack into a parallel
> value to return and it has TImode when misaligned access is supported,
> however TImode required 16-byte align, but it only 8-byte align, so it go to
> the misaligned stores handling, then it will try to generate move
> instruction from a PARALLEL value.
> 
> Tested on following target without introduced new reguression:
>   - riscv32/riscv64 elf
>   - x86_64-linux
>   - arm-eabi

riscv doesn't seem to have movmisalign and I don't see why movmisalign
should not support a TImode parallel RHS so at least you should
put this check after the icode != CODE_FOR_nothing check?

Also wouldn't it be better to support PARALLEL from within
store_bit_field?  After all this is a misaligned access and
since you didn't quote the RTL involved I'm guessing that
emit_group_store knows nothing about this fact.

Richard.

> gcc/ChangeLog:
> 
> 	PR target/96759
> 	* expr.c (expand_assignment): Handle misaligned stores with PARALLEL
> 	value.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR target/96759
> 	* g++.target/riscv/pr96759.C: New.
> 	* gcc.target/riscv/pr96759.c: New.
> ---
>  gcc/expr.c                               |  9 ++++++++-
>  gcc/testsuite/g++.target/riscv/pr96759.C |  8 ++++++++
>  gcc/testsuite/gcc.target/riscv/pr96759.c | 13 +++++++++++++
>  3 files changed, 29 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/g++.target/riscv/pr96759.C
>  create mode 100644 gcc/testsuite/gcc.target/riscv/pr96759.c
> 
> diff --git a/gcc/expr.c b/gcc/expr.c
> index 1a15f24b3979..cc2bc52c457f 100644
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -5173,7 +5173,14 @@ expand_assignment (tree to, tree from, bool nontemporal)
>        if (TREE_CODE (to) == MEM_REF && REF_REVERSE_STORAGE_ORDER (to))
>  	reg = flip_storage_order (mode, reg);
>  
> -      if (icode != CODE_FOR_nothing)
> +      if (GET_CODE (reg) == PARALLEL)
> +	{
> +	  push_temp_slots ();
> +	  emit_group_store (mem, reg, TREE_TYPE (from),
> +			    int_size_in_bytes (TREE_TYPE (from)));
> +	  pop_temp_slots ();
> +	}
> +      else if (icode != CODE_FOR_nothing)
>  	{
>  	  class expand_operand ops[2];
>  
> diff --git a/gcc/testsuite/g++.target/riscv/pr96759.C b/gcc/testsuite/g++.target/riscv/pr96759.C
> new file mode 100644
> index 000000000000..673999a4baf7
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/riscv/pr96759.C
> @@ -0,0 +1,8 @@
> +/* { dg-options "-mno-strict-align -std=gnu++17" } */
> +/* { dg-do compile } */
> +struct S {
> +  int a;
> +  double b;
> +};
> +S GetNumbers();
> +auto [globalC, globalD] = GetNumbers();
> diff --git a/gcc/testsuite/gcc.target/riscv/pr96759.c b/gcc/testsuite/gcc.target/riscv/pr96759.c
> new file mode 100644
> index 000000000000..621c39196fca
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/pr96759.c
> @@ -0,0 +1,13 @@
> +/* { dg-options "-mno-strict-align" } */
> +/* { dg-do compile } */
> +
> +struct S {
> +  int a;
> +  double b;
> +};
> +struct S GetNumbers();
> +struct S g;
> +
> +void foo(){
> +  g = GetNumbers();
> +}
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)