[patch] Fix wrong code for return of small aggregates on big-endian

[patch] Fix wrong code for return of small aggregates on big-endian

[Eric Botcazou]

[-- 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;

Re: [patch] Fix wrong code for return of small aggregates on big-endian

[Richard Biener]

On Mon, Jan 9, 2017 at 11:43 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> 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?

Ok for trunk and branches after a short burn-in.

Thanks,
Richard.

>
> 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

Re: [patch] Fix wrong code for return of small aggregates on big-endian

[Christophe Lyon]

On 9 January 2017 at 12:14, Richard Biener <richard.guenther@gmail.com> wrote:
> On Mon, Jan 9, 2017 at 11:43 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> 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?
>
> Ok for trunk and branches after a short burn-in.
>

Hi Eric,

I have noticed new failures after this commit (r244249).
g++.dg/opt/call3.C fails at execution on armeb targets
g++.dg/opt/call2.C fails at execution on aarch64_be

Christophe




> Thanks,
> Richard.
>
>>
>> 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

Re: [patch] Fix wrong code for return of small aggregates on big-endian

[Eric Botcazou]

> I have noticed new failures after this commit (r244249).
> g++.dg/opt/call3.C fails at execution on armeb targets
> g++.dg/opt/call2.C fails at execution on aarch64_be

They are new testcases: can you find out whether they pass before the patch?

-- 
Eric Botcazou

Re: [patch] Fix wrong code for return of small aggregates on big-endian

[Christophe Lyon]

On 10 January 2017 at 09:58, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> I have noticed new failures after this commit (r244249).
>> g++.dg/opt/call3.C fails at execution on armeb targets
>> g++.dg/opt/call2.C fails at execution on aarch64_be
>
> They are new testcases: can you find out whether they pass before the patch?
>
They pass before the patch (I only checked armeb).

> --
> Eric Botcazou

Re: [patch] Fix wrong code for return of small aggregates on big-endian

[Eric Botcazou]

> They pass before the patch (I only checked armeb).

Thanks, I see what's going on, but can you post the configure line of armeb?

-- 
Eric Botcazou

Re: [patch] Fix wrong code for return of small aggregates on big-endian

[Christophe Lyon]

On 10 January 2017 at 11:26, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> They pass before the patch (I only checked armeb).
>
> Thanks, I see what's going on, but can you post the configure line of armeb?
>
Sure, it is:
--target=armeb-none-linux-gnueabihf  --with-float=hard --with-mode=arm
--with-cpu=cortex-a9 --with-fpu=neon


> --
> Eric Botcazou

Re: [patch] Fix wrong code for return of small aggregates on big-endian

[Eric Botcazou]

> They pass before the patch (I only checked armeb).

I think that's not true for aarch64_be though, since the patch doesn't change 
code generation for this target.  But I'll fix that too.

-- 
Eric Botcazou

Re: [patch] Fix wrong code for return of small aggregates on big-endian

[Eric Botcazou]

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

> I have noticed new failures after this commit (r244249).
> g++.dg/opt/call3.C fails at execution on armeb targets
> g++.dg/opt/call2.C fails at execution on aarch64_be

It turns out that there is already a big-endian adjustment a few lines above:

      /* If the value has a record type and an integral mode then, if BITSIZE
	 is narrower than this mode and this is for big-endian data, we must
        first put the value into the low-order bits.  Moreover, the field may
	 be not aligned on a byte boundary; in this case, if it has reverse
	storage order, it needs to be accessed as a scalar field with reverse
	 storage order and we must first put the value into target order.  */
      if (TREE_CODE (TREE_TYPE (exp)) == RECORD_TYPE
	  && GET_MODE_CLASS (GET_MODE (temp)) == MODE_INT)
	{
	  HOST_WIDE_INT size = GET_MODE_BITSIZE (GET_MODE (temp));

	  reverse = TYPE_REVERSE_STORAGE_ORDER (TREE_TYPE (exp));

	  if (reverse)
	    temp = flip_storage_order (GET_MODE (temp), temp);

	  if (bitsize < size
	      && reverse ? !BYTES_BIG_ENDIAN : BYTES_BIG_ENDIAN)
	    temp = expand_shift (RSHIFT_EXPR, GET_MODE (temp), temp,
				 size - bitsize, NULL_RTX, 1);
	}

and adding the extraction leads to a double adjustment on armeb, whereas no 
adjustment is still applied to aarch64_be...

That's why the attached patch merges the second adjustment in the first one by 
moving up the code fetching the return value from the registers; this ensures 
that only one adjustment is ever applied and that it is applied only on left- 
justified values.  But a final extraction (without big-endian adjustment) is 
still needed for small BLKmode values, otherwise store_bit_field aborts...

Tested on the same platforms as the original patch, applied as obvious.


        * expr.c (store_field): In the bitfield case, fetch the return value
	from the registers before applying a single big-endian adjustment.
	Always do a final load for a BLKmode value not larger than a word.

-- 
Eric Botcazou

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

Index: expr.c
===================================================================
--- expr.c	(revision 244258)
+++ expr.c	(working copy)
@@ -6832,13 +6832,36 @@ store_field (rtx target, HOST_WIDE_INT b
 
       temp = expand_normal (exp);
 
-      /* If the value has a record type and an integral mode then, if BITSIZE
-	 is narrower than this mode and this is for big-endian data, we must
-	 first put the value into the low-order bits.  Moreover, the field may
-	 be not aligned on a byte boundary; in this case, if it has reverse
-	 storage order, it needs to be accessed as a scalar field with reverse
-	 storage order and we must first put the value into target order.  */
-      if (TREE_CODE (TREE_TYPE (exp)) == RECORD_TYPE
+      /* Handle calls that return values in multiple non-contiguous locations.
+	 The Irix 6 ABI has examples of this.  */
+      if (GET_CODE (temp) == PARALLEL)
+	{
+	  HOST_WIDE_INT size = int_size_in_bytes (TREE_TYPE (exp));
+	  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;
+	}
+
+      /* 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;
+	}
+
+      /* If the value has aggregate type and an integral mode then, if BITSIZE
+	 is narrower than this mode and this is for big-endian data, we first
+	 need to put the value into the low-order bits for store_bit_field,
+	 except when MODE is BLKmode and BITSIZE larger than the word size
+	 (see the handling of fields larger than a word in store_bit_field).
+	 Moreover, the field may be not aligned on a byte boundary; in this
+	 case, if it has reverse storage order, it needs to be accessed as a
+	 scalar field with reverse storage order and we must first put the
+	 value into target order.  */
+      if (AGGREGATE_TYPE_P (TREE_TYPE (exp))
 	  && GET_MODE_CLASS (GET_MODE (temp)) == MODE_INT)
 	{
 	  HOST_WIDE_INT size = GET_MODE_BITSIZE (GET_MODE (temp));
@@ -6849,7 +6872,8 @@ store_field (rtx target, HOST_WIDE_INT b
 	    temp = flip_storage_order (GET_MODE (temp), temp);
 
 	  if (bitsize < size
-	      && reverse ? !BYTES_BIG_ENDIAN : BYTES_BIG_ENDIAN)
+	      && reverse ? !BYTES_BIG_ENDIAN : BYTES_BIG_ENDIAN
+	      && !(mode == BLKmode && bitsize > BITS_PER_WORD))
 	    temp = expand_shift (RSHIFT_EXPR, GET_MODE (temp), temp,
 				 size - bitsize, NULL_RTX, 1);
 	}
@@ -6859,12 +6883,10 @@ store_field (rtx target, HOST_WIDE_INT b
 	  && mode != TYPE_MODE (TREE_TYPE (exp)))
 	temp = convert_modes (mode, TYPE_MODE (TREE_TYPE (exp)), temp, 1);
 
-      /* If TEMP is not a PARALLEL (see below) and its mode and that of TARGET
-	 are both BLKmode, both must be in memory and BITPOS must be aligned
-	 on a byte boundary.  If so, we simply do a block copy.  Likewise for
-	 a BLKmode-like TARGET.  */
-      if (GET_CODE (temp) != PARALLEL
-	  && GET_MODE (temp) == BLKmode
+      /* If the mode of TEMP and TARGET is BLKmode, both must be in memory
+	 and BITPOS must be aligned on a byte boundary.  If so, we simply do
+	 a block copy.  Likewise for a BLKmode-like TARGET.  */
+      if (GET_MODE (temp) == BLKmode
 	  && (GET_MODE (target) == BLKmode
 	      || (MEM_P (target)
 		  && GET_MODE_CLASS (GET_MODE (target)) == MODE_INT
@@ -6883,31 +6905,9 @@ store_field (rtx target, HOST_WIDE_INT b
 	  return const0_rtx;
 	}
 
-      /* Handle calls that return values in multiple non-contiguous locations.
-	 The Irix 6 ABI has examples of this.  */
-      if (GET_CODE (temp) == PARALLEL)
-	{
-	  HOST_WIDE_INT size = int_size_in_bytes (TREE_TYPE (exp));
-	  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;
-	}
-
-      /* 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)
+      /* If the mode of TEMP is still BLKmode and BITSIZE not larger than the
+	 word size, we need to load the value (see again store_bit_field).  */
+      if (GET_MODE (temp) == BLKmode && bitsize <= BITS_PER_WORD)
 	{
 	  machine_mode temp_mode = smallest_mode_for_size (bitsize, MODE_INT);
 	  temp = extract_bit_field (temp, bitsize, 0, 1, NULL_RTX, temp_mode,