public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch] Fix wrong code for return of small aggregates on big-endian
@ 2017-01-09 10:46 Eric Botcazou
  2017-01-09 11:14 ` Richard Biener
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Botcazou @ 2017-01-09 10:46 UTC (permalink / raw)
  To: gcc-patches

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

Hi,

this is a regression present on all active branches for big-endian targets 
returning small aggregate types in registers under certain circumstances and 
when optimization is enabled: when the bitfield path of store_field is taken, 
the function ends up calling store_bit_field to store the value.  Now the 
behavior of store_bit_field is awkward when the mode is BLKmode: it always 
takes its value from the lsb up to the word size but expects it left justified 
beyond it (see expmed.c:890 and below) and I missed that when I got rid of the 
stack temporaries that were originally generated in that case.

Of course that's OK for little-endian targets but not for big-endian targets, 
and I have a couple of C++ testcases exposing the issue on SPARC 64-bit and a 
couple of Ada testcases exposing the issue on PowerPC with the SVR4 ABI (the 
Linux ABI is immune since it always returns on the stack); I think they cover 
all the cases in the problematic code.

The attached fix was tested on a bunch of platforms: x86/Linux, x86-64/Linux, 
PowerPC/Linux, PowerPC64/Linux, PowerPC/VxWorks, Aarch64/Linux, SPARC/Solaris 
and SPARC64/Solaris with no regressions.  OK for the mainline? other branches?


2017-01-09  Eric Botcazou  <ebotcazou@adacore.com>

	* expr.c (store_field): In the bitfield case, if the value comes from
	a function call and is of an aggregate type returned in registers, do
	not modify the field mode; extract the value in all cases if the mode
	is BLKmode and the size is not larger than a word.


2017-01-09  Eric Botcazou  <ebotcazou@adacore.com>

	* g++.dg/opt/call2.C: New test.
	* g++.dg/opt/call3.C: Likewise.
	* gnat.dg/array26.adb: New test.
	* gnat.dg/array26_pkg.ad[sb]: New helper.
	* gnat.dg/array27.adb: New test.
	* gnat.dg/array27_pkg.ad[sb]: New helper.
	* gnat.dg/array28.adb: New test.
	* gnat.dg/array28_pkg.ad[sb]: New helper.

-- 
Eric Botcazou

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

Index: expr.c
===================================================================
--- expr.c	(revision 244194)
+++ expr.c	(working copy)
@@ -6888,33 +6888,30 @@ store_field (rtx target, HOST_WIDE_INT b
       if (GET_CODE (temp) == PARALLEL)
 	{
 	  HOST_WIDE_INT size = int_size_in_bytes (TREE_TYPE (exp));
-	  rtx temp_target;
-	  if (mode == BLKmode || mode == VOIDmode)
-	    mode = smallest_mode_for_size (size * BITS_PER_UNIT, MODE_INT);
-	  temp_target = gen_reg_rtx (mode);
+	  machine_mode temp_mode
+	    = smallest_mode_for_size (size * BITS_PER_UNIT, MODE_INT);
+	  rtx temp_target = gen_reg_rtx (temp_mode);
 	  emit_group_store (temp_target, temp, TREE_TYPE (exp), size);
 	  temp = temp_target;
 	}
-      else if (mode == BLKmode)
+
+      /* Handle calls that return BLKmode values in registers.  */
+      else if (mode == BLKmode && REG_P (temp) && TREE_CODE (exp) == CALL_EXPR)
+	{
+	  rtx temp_target = gen_reg_rtx (GET_MODE (temp));
+	  copy_blkmode_from_reg (temp_target, temp, TREE_TYPE (exp));
+	  temp = temp_target;
+	}
+
+      /* The behavior of store_bit_field is awkward when mode is BLKmode:
+	 it always takes its value from the lsb up to the word size but
+	 expects it left justified beyond it.  At this point TEMP is left
+	 justified so extract the value in the former case.  */
+      if (mode == BLKmode && bitsize <= BITS_PER_WORD)
 	{
-	  /* Handle calls that return BLKmode values in registers.  */
-	  if (REG_P (temp) && TREE_CODE (exp) == CALL_EXPR)
-	    {
-	      rtx temp_target = gen_reg_rtx (GET_MODE (temp));
-	      copy_blkmode_from_reg (temp_target, temp, TREE_TYPE (exp));
-	      temp = temp_target;
-	    }
-	  else
-	    {
-	      HOST_WIDE_INT size = int_size_in_bytes (TREE_TYPE (exp));
-	      rtx temp_target;
-	      mode = smallest_mode_for_size (size * BITS_PER_UNIT, MODE_INT);
-	      temp_target = gen_reg_rtx (mode);
-	      temp_target
-	        = extract_bit_field (temp, size * BITS_PER_UNIT, 0, 1,
-				     temp_target, mode, mode, false);
-	      temp = temp_target;
-	    }
+	  machine_mode temp_mode = smallest_mode_for_size (bitsize, MODE_INT);
+	  temp = extract_bit_field (temp, bitsize, 0, 1, NULL_RTX, temp_mode,
+				    temp_mode, false);
 	}
 
       /* Store the value in the bitfield.  */

[-- Attachment #3: call2.C --]
[-- Type: text/x-c++src, Size: 403 bytes --]

// { dg-do run }
// { dg-options "-O" }

struct Foo
{
  Foo() : a(1), b(1), c('a') {}
  int a;
  int b;
  char c;
};

static Foo copy_foo(Foo) __attribute__((noinline, noclone));

static Foo copy_foo(Foo A)
{
  return A;
}

struct Bar : Foo
{
  Bar(Foo t) : Foo(copy_foo(t)) {}
};

Foo F;

int main (void)
{
  Bar B (F);

  if (B.a != 1 || B.b != 1 || B.c != 'a')
    __builtin_abort ();

  return 0;
}

[-- Attachment #4: call3.C --]
[-- Type: text/x-c++src, Size: 382 bytes --]

// { dg-do run }
// { dg-options "-O" }

struct Foo
{
  Foo() : a(1), c('a') {}
  short int a;
  char c;
};

static Foo copy_foo(Foo) __attribute__((noinline, noclone));

static Foo copy_foo(Foo A)
{
  return A;
}

struct Bar : Foo
{
  Bar(Foo t) : Foo(copy_foo(t)) {}
};

Foo F;

int main (void)
{
  Bar B (F);

  if (B.a != 1 || B.c != 'a')
    __builtin_abort ();

  return 0;
}

[-- Attachment #5: array26.adb --]
[-- Type: text/x-adasrc, Size: 357 bytes --]

-- { dg-do run }
-- { dg-options "-O" }

with Array26_Pkg; use Array26_Pkg;

procedure Array26 is

  function Get return Outer_type is
    Ret : Outer_Type;
  begin
    Ret (Inner_Type'Range) := F;
    return Ret;
  end;

  A : Outer_Type := Get;
  B : Inner_Type := A (Inner_Type'Range);

begin
  if B /= "123" then
    raise Program_Error;
  end if;
end;

[-- Attachment #6: array26_pkg.adb --]
[-- Type: text/x-adasrc, Size: 114 bytes --]

package body Array26_Pkg is

  function F return Inner_Type is
  begin
    return "123";
  end;

end Array26_Pkg;

[-- Attachment #7: array27_pkg.ads --]
[-- Type: text/x-adasrc, Size: 157 bytes --]

package Array27_Pkg is

  subtype Outer_Type is String (1 .. 8);
  subtype Inner_Type is String (1 .. 3);

  function F return Inner_Type;

end Array27_Pkg;

[-- Attachment #8: array26_pkg.ads --]
[-- Type: text/x-adasrc, Size: 157 bytes --]

package Array26_Pkg is

  subtype Outer_Type is String (1 .. 4);
  subtype Inner_Type is String (1 .. 3);

  function F return Inner_Type;

end Array26_Pkg;

[-- Attachment #9: array28.adb --]
[-- Type: text/x-adasrc, Size: 359 bytes --]

-- { dg-do run }
-- { dg-options "-O" }

with Array28_Pkg; use Array28_Pkg;

procedure Array28 is

  function Get return Outer_type is
    Ret : Outer_Type;
  begin
    Ret (Inner_Type'Range) := F;
    return Ret;
  end;

  A : Outer_Type := Get;
  B : Inner_Type := A (Inner_Type'Range);

begin
  if B /= "12345" then
    raise Program_Error;
  end if;
end;

[-- Attachment #10: array27.adb --]
[-- Type: text/x-adasrc, Size: 359 bytes --]

-- { dg-do run }
-- { dg-options "-O" }

with Array27_Pkg; use Array27_Pkg;

procedure Array27 is

  function Get return Outer_type is
    Ret : Outer_Type;
  begin
    Ret (Inner_Type'Range) := F;
    return Ret;
  end;

  A : Outer_Type := Get;
  B : Inner_Type := A (Inner_Type'Range);

begin
    if B /= "123" then
    raise Program_Error;
  end if;
end;

[-- Attachment #11: array27_pkg.adb --]
[-- Type: text/x-adasrc, Size: 114 bytes --]

package body Array27_Pkg is

  function F return Inner_Type is
  begin
    return "123";
  end;

end Array27_Pkg;

[-- Attachment #12: array28_pkg.adb --]
[-- Type: text/x-adasrc, Size: 116 bytes --]

package body Array28_Pkg is

  function F return Inner_Type is
  begin
    return "12345";
  end;

end Array28_Pkg;

[-- Attachment #13: array28_pkg.ads --]
[-- Type: text/x-adasrc, Size: 157 bytes --]

package Array28_Pkg is

  subtype Outer_Type is String (1 .. 8);
  subtype Inner_Type is String (1 .. 5);

  function F return Inner_Type;

end Array28_Pkg;

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-01-10 23:06 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-09 10:46 [patch] Fix wrong code for return of small aggregates on big-endian Eric Botcazou
2017-01-09 11:14 ` Richard Biener
2017-01-10  8:01   ` Christophe Lyon
2017-01-10  8:58     ` Eric Botcazou
2017-01-10 10:20       ` Christophe Lyon
2017-01-10 10:27         ` Eric Botcazou
2017-01-10 11:09           ` Christophe Lyon
2017-01-10 12:14         ` Eric Botcazou
2017-01-10 23:06     ` Eric Botcazou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).