[PATCH 1/2] arm: correctly handle zero-sized bit-fields in AAPCS [PR102024]

[PATCH 1/2] arm: correctly handle zero-sized bit-fields in AAPCS [PR102024]

[Richard Earnshaw]

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


On arm the AAPCS states that an HFA is determined by the 'shape' of
the object after layout has been completed, so anything that adds no
members and does not cause the layout to be modified should be ignored
for the purposes of determining which registers are used for parameter
passing.

A zero-sized bit-field falls into this category.  This was not handled
correctly for C structs and in G++-11 only handled correctly because
such fields were eliminated early by the front end.

gcc/ChangeLog:

	PR target/102024
	* config/arm/arm.cc (aapcs_vfp_sub_candidate): Handle zero-sized
	bit-fields.  Detect cases where a warning may be needed.
	(aapcs_vfp_is_call_or_return_candidate): Emit a note if
	a zero-sized bit-field has caused parameter passing to change.

gcc/testsuite/ChangeLog:

	PR target/102024
	* gcc.target/arm/aapcs/vfp26.c: New test.
---
 gcc/config/arm/arm.cc                      | 35 ++++++++++++++++++++--
 gcc/testsuite/gcc.target/arm/aapcs/vfp26.c | 31 +++++++++++++++++++
 2 files changed, 63 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/arm/aapcs/vfp26.c


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-arm-correctly-handle-zero-sized-bit-fields-in-AAPCS-.patch --]
[-- Type: text/x-patch; name="0001-arm-correctly-handle-zero-sized-bit-fields-in-AAPCS-.patch", Size: 3975 bytes --]

diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc
index e062361b985..43b775f6918 100644
--- a/gcc/config/arm/arm.cc
+++ b/gcc/config/arm/arm.cc
@@ -6274,6 +6274,7 @@ aapcs_vfp_cum_init (CUMULATIVE_ARGS *pcum  ATTRIBUTE_UNUSED,
       a HFA or HVA.  */
 const unsigned int WARN_PSABI_EMPTY_CXX17_BASE = 1U << 0;
 const unsigned int WARN_PSABI_NO_UNIQUE_ADDRESS = 1U << 1;
+const unsigned int WARN_PSABI_ZERO_WIDTH_BITFIELD = 1U << 2;
 
 /* Walk down the type tree of TYPE counting consecutive base elements.
    If *MODEP is VOIDmode, then set it to the first valid floating point
@@ -6426,6 +6427,28 @@ aapcs_vfp_sub_candidate (const_tree type, machine_mode *modep,
 		    continue;
 		  }
 	      }
+	    /* A zero-width bitfield may affect layout in some
+	       circumstances, but adds no members.  The determination
+	       of whether or not a type is an HFA is performed after
+	       layout is complete, so if the type still looks like an
+	       HFA afterwards, it is still classed as one.  This is
+	       potentially an ABI break for the hard-float ABI.  */
+	    else if (DECL_BIT_FIELD (field)
+		     && integer_zerop (DECL_SIZE (field)))
+	      {
+		/* Prior to GCC-12 these fields were striped early,
+		   hiding them from the back-end entirely and
+		   resulting in the correct behaviour for argument
+		   passing.  Simulate that old behaviour without
+		   generating a warning.  */
+		if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))
+		  continue;
+		if (warn_psabi_flags)
+		  {
+		    *warn_psabi_flags |= WARN_PSABI_ZERO_WIDTH_BITFIELD;
+		    continue;
+		  }
+	      }
 
 	    sub_count = aapcs_vfp_sub_candidate (TREE_TYPE (field), modep,
 						 warn_psabi_flags);
@@ -6538,8 +6561,10 @@ aapcs_vfp_is_call_or_return_candidate (enum arm_pcs pcs_variant,
 	      && ((alt = aapcs_vfp_sub_candidate (type, &new_mode, NULL))
 		  != ag_count))
 	    {
-	      const char *url
+	      const char *url10
 		= CHANGES_ROOT_URL "gcc-10/changes.html#empty_base";
+	      const char *url12
+		= CHANGES_ROOT_URL "gcc-12/changes.html#empty_base";
 	      gcc_assert (alt == -1);
 	      last_reported_type_uid = uid;
 	      /* Use TYPE_MAIN_VARIANT to strip any redundant const
@@ -6548,12 +6573,16 @@ aapcs_vfp_is_call_or_return_candidate (enum arm_pcs pcs_variant,
 		inform (input_location, "parameter passing for argument of "
 			"type %qT with %<[[no_unique_address]]%> members "
 			"changed %{in GCC 10.1%}",
-			TYPE_MAIN_VARIANT (type), url);
+			TYPE_MAIN_VARIANT (type), url10);
 	      else if (warn_psabi_flags & WARN_PSABI_EMPTY_CXX17_BASE)
 		inform (input_location, "parameter passing for argument of "
 			"type %qT when C++17 is enabled changed to match "
 			"C++14 %{in GCC 10.1%}",
-			TYPE_MAIN_VARIANT (type), url);
+			TYPE_MAIN_VARIANT (type), url10);
+	      else if (warn_psabi_flags & WARN_PSABI_ZERO_WIDTH_BITFIELD)
+		inform (input_location, "parameter passing for argument of "
+			"type %qT changed %{in GCC 12.1%}",
+			TYPE_MAIN_VARIANT (type), url12);
 	    }
 	  *count = ag_count;
 	}
diff --git a/gcc/testsuite/gcc.target/arm/aapcs/vfp26.c b/gcc/testsuite/gcc.target/arm/aapcs/vfp26.c
new file mode 100644
index 00000000000..9b1e8aa39d6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/aapcs/vfp26.c
@@ -0,0 +1,31 @@
+/* Test AAPCS layout (VFP variant) */
+
+/* { dg-do run { target arm_eabi } } */
+/* { dg-require-effective-target arm_hard_vfp_ok } */
+/* { dg-require-effective-target arm32 } */
+/* { dg-options "-O -mfpu=vfp -mfloat-abi=hard" } */
+
+#ifndef IN_FRAMEWORK
+#define VFP
+#define TESTFILE "vfp26.c"
+
+/* Anonymous bitfields do not add members; if they do not change the layout
+   then the end result may still be an HFA.  */
+struct z
+{
+  float a;
+  int :0;
+  float b;
+};
+
+struct z a = { 5.0f, 6.0f };
+struct z b = { 9.0f, 10.0f };
+
+#define MYFUNCTYPE struct z
+
+#include "abitest.h"
+#else
+  ARG(int, 7, R0)
+  ARG(struct z, a, S0)
+  LAST_ARG(struct z, b, S2)
+#endif

[PATCH 2/2] aarch64: correctly handle zero-sized bit-fields in AAPCS64 [PR102024]

[Richard Earnshaw]

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


On aarch64 the AAPCS64 states that an HFA is determined by the 'shape' of
the object after layout has been completed, so anything that adds no
members and does not cause the layout to be modified should be ignored
for the purposes of determining which registers are used for parameter
passing.

A zero-sized bit-field falls into this category.  This was not handled
correctly for C structs and in G++-11 only handled correctly because
such fields were eliminated early by the front end.

gcc/ChangeLog:

	PR target/102024
	* config/aarch64/aarch64.cc (aapcs_vfp_sub_candidate): Handle
	zero-sized bit-fields.  Detect cases where a warning may be needed.
	(aarch64_vfp_is_call_or_return_candidate): Emit a note if a
	zero-sized bit-field has caused parameter passing to change.

gcc/testsuite/ChangeLog:

	* gcc.target/aarch64/aapcs64/test_28.c: New test.
---
 gcc/config/aarch64/aarch64.cc                 | 35 +++++++++++++++++--
 .../gcc.target/aarch64/aapcs64/test_28.c      | 28 +++++++++++++++
 2 files changed, 60 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/test_28.c


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-aarch64-correctly-handle-zero-sized-bit-fields-in-AA.patch --]
[-- Type: text/x-patch; name="0002-aarch64-correctly-handle-zero-sized-bit-fields-in-AA.patch", Size: 3935 bytes --]

diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index dbeaaf484db..296f393cf39 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -19355,6 +19355,7 @@ aarch64_member_type_forces_blk (const_tree field_or_array, machine_mode mode)
       a HFA or HVA.  */
 const unsigned int WARN_PSABI_EMPTY_CXX17_BASE = 1U << 0;
 const unsigned int WARN_PSABI_NO_UNIQUE_ADDRESS = 1U << 1;
+const unsigned int WARN_PSABI_ZERO_WIDTH_BITFIELD = 1U << 2;
 
 /* Walk down the type tree of TYPE counting consecutive base elements.
    If *MODEP is VOIDmode, then set it to the first valid floating point
@@ -19511,6 +19512,28 @@ aapcs_vfp_sub_candidate (const_tree type, machine_mode *modep,
 		    continue;
 		  }
 	      }
+	    /* A zero-width bitfield may affect layout in some
+	       circumstances, but adds no members.  The determination
+	       of whether or not a type is an HFA is performed after
+	       layout is complete, so if the type still looks like an
+	       HFA afterwards, it is still classed as one.  This is
+	       potentially an ABI break for the hard-float ABI.  */
+	    else if (DECL_BIT_FIELD (field)
+		     && integer_zerop (DECL_SIZE (field)))
+	      {
+		/* Prior to GCC-12 these fields were striped early,
+		   hiding them from the back-end entirely and
+		   resulting in the correct behaviour for argument
+		   passing.  Simulate that old behaviour without
+		   generating a warning.  */
+		if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))
+		  continue;
+		if (warn_psabi_flags)
+		  {
+		    *warn_psabi_flags |= WARN_PSABI_ZERO_WIDTH_BITFIELD;
+		    continue;
+		  }
+	      }
 
 	    sub_count = aapcs_vfp_sub_candidate (TREE_TYPE (field), modep,
 						 warn_psabi_flags);
@@ -19711,8 +19734,10 @@ aarch64_vfp_is_call_or_return_candidate (machine_mode mode,
 	      && ((alt = aapcs_vfp_sub_candidate (type, &new_mode, NULL))
 		  != ag_count))
 	    {
-	      const char *url
+	      const char *url10
 		= CHANGES_ROOT_URL "gcc-10/changes.html#empty_base";
+	      const char *url12
+		= CHANGES_ROOT_URL "gcc-12/changes.html#empty_base";
 	      gcc_assert (alt == -1);
 	      last_reported_type_uid = uid;
 	      /* Use TYPE_MAIN_VARIANT to strip any redundant const
@@ -19721,12 +19746,16 @@ aarch64_vfp_is_call_or_return_candidate (machine_mode mode,
 		inform (input_location, "parameter passing for argument of "
 			"type %qT with %<[[no_unique_address]]%> members "
 			"changed %{in GCC 10.1%}",
-			TYPE_MAIN_VARIANT (type), url);
+			TYPE_MAIN_VARIANT (type), url10);
 	      else if (warn_psabi_flags & WARN_PSABI_EMPTY_CXX17_BASE)
 		inform (input_location, "parameter passing for argument of "
 			"type %qT when C++17 is enabled changed to match "
 			"C++14 %{in GCC 10.1%}",
-			TYPE_MAIN_VARIANT (type), url);
+			TYPE_MAIN_VARIANT (type), url10);
+	      else if (warn_psabi_flags & WARN_PSABI_ZERO_WIDTH_BITFIELD)
+		inform (input_location, "parameter passing for argument of "
+			"type %qT changed %{in GCC 12.1%}",
+			TYPE_MAIN_VARIANT (type), url12);
 	    }
 
 	  if (is_ha != NULL) *is_ha = true;
diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_28.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_28.c
new file mode 100644
index 00000000000..951057b3509
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_28.c
@@ -0,0 +1,28 @@
+/* Test AAPCS64 layout for HFA with zero-sized bit-field.  */
+
+/* { dg-do run { target aarch64*-*-* } } */
+
+#ifndef IN_FRAMEWORK
+#define VFP
+#define TESTFILE "test_28.c"
+
+/* Anonymous bitfields do not add members; if they do not change the layout
+   then the end result may still be an HFA.  */
+struct z
+{
+  float a;
+  int :0;
+  float b;
+};
+
+struct z a = { 5.0f, 6.0f };
+struct z b = { 9.0f, 10.0f };
+
+#define MYFUNCTYPE struct z
+
+#include "abitest.h"
+#else
+  ARG(int, 7, W0)
+  ARG(struct z, a, S0)
+  LAST_ARG(struct z, b, S2)
+#endif

Re: [PATCH 1/2] arm: correctly handle zero-sized bit-fields in AAPCS [PR102024]

[Jakub Jelinek]

On Tue, Mar 29, 2022 at 04:32:10PM +0100, Richard Earnshaw wrote:
> 
> On arm the AAPCS states that an HFA is determined by the 'shape' of
> the object after layout has been completed, so anything that adds no
> members and does not cause the layout to be modified should be ignored
> for the purposes of determining which registers are used for parameter
> passing.
> 
> A zero-sized bit-field falls into this category.  This was not handled
> correctly for C structs and in G++-11 only handled correctly because
> such fields were eliminated early by the front end.
> 
> gcc/ChangeLog:
> 
> 	PR target/102024
> 	* config/arm/arm.cc (aapcs_vfp_sub_candidate): Handle zero-sized
> 	bit-fields.  Detect cases where a warning may be needed.
> 	(aapcs_vfp_is_call_or_return_candidate): Emit a note if
> 	a zero-sized bit-field has caused parameter passing to change.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR target/102024
> 	* gcc.target/arm/aapcs/vfp26.c: New test.
> ---
>  gcc/config/arm/arm.cc                      | 35 ++++++++++++++++++++--
>  gcc/testsuite/gcc.target/arm/aapcs/vfp26.c | 31 +++++++++++++++++++
>  2 files changed, 63 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/arm/aapcs/vfp26.c
> 

> diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc
> index e062361b985..43b775f6918 100644
> --- a/gcc/config/arm/arm.cc
> +++ b/gcc/config/arm/arm.cc
> @@ -6274,6 +6274,7 @@ aapcs_vfp_cum_init (CUMULATIVE_ARGS *pcum  ATTRIBUTE_UNUSED,
>        a HFA or HVA.  */
>  const unsigned int WARN_PSABI_EMPTY_CXX17_BASE = 1U << 0;
>  const unsigned int WARN_PSABI_NO_UNIQUE_ADDRESS = 1U << 1;
> +const unsigned int WARN_PSABI_ZERO_WIDTH_BITFIELD = 1U << 2;
>  
>  /* Walk down the type tree of TYPE counting consecutive base elements.
>     If *MODEP is VOIDmode, then set it to the first valid floating point
> @@ -6426,6 +6427,28 @@ aapcs_vfp_sub_candidate (const_tree type, machine_mode *modep,
>  		    continue;
>  		  }
>  	      }
> +	    /* A zero-width bitfield may affect layout in some
> +	       circumstances, but adds no members.  The determination
> +	       of whether or not a type is an HFA is performed after
> +	       layout is complete, so if the type still looks like an
> +	       HFA afterwards, it is still classed as one.  This is
> +	       potentially an ABI break for the hard-float ABI.  */
> +	    else if (DECL_BIT_FIELD (field)
> +		     && integer_zerop (DECL_SIZE (field)))
> +	      {
> +		/* Prior to GCC-12 these fields were striped early,
> +		   hiding them from the back-end entirely and
> +		   resulting in the correct behaviour for argument
> +		   passing.  Simulate that old behaviour without
> +		   generating a warning.  */
> +		if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))
> +		  continue;
> +		if (warn_psabi_flags)
> +		  {
> +		    *warn_psabi_flags |= WARN_PSABI_ZERO_WIDTH_BITFIELD;
> +		    continue;
> +		  }
> +	      }
>  
>  	    sub_count = aapcs_vfp_sub_candidate (TREE_TYPE (field), modep,
>  						 warn_psabi_flags);
> @@ -6538,8 +6561,10 @@ aapcs_vfp_is_call_or_return_candidate (enum arm_pcs pcs_variant,
>  	      && ((alt = aapcs_vfp_sub_candidate (type, &new_mode, NULL))
>  		  != ag_count))
>  	    {
> -	      const char *url
> +	      const char *url10
>  		= CHANGES_ROOT_URL "gcc-10/changes.html#empty_base";
> +	      const char *url12
> +		= CHANGES_ROOT_URL "gcc-12/changes.html#empty_base";

This should be
		= CHANGES_ROOT_URL "gcc-12/changes.html#zero_width_bitfields";
instead.

Otherwise LGTM.

	Jakub

Re: [PATCH 2/2] aarch64: correctly handle zero-sized bit-fields in AAPCS64 [PR102024]

[Jakub Jelinek]

On Tue, Mar 29, 2022 at 04:32:11PM +0100, Richard Earnshaw wrote:
> 
> On aarch64 the AAPCS64 states that an HFA is determined by the 'shape' of
> the object after layout has been completed, so anything that adds no
> members and does not cause the layout to be modified should be ignored
> for the purposes of determining which registers are used for parameter
> passing.
> 
> A zero-sized bit-field falls into this category.  This was not handled
> correctly for C structs and in G++-11 only handled correctly because
> such fields were eliminated early by the front end.
> 
> gcc/ChangeLog:
> 
> 	PR target/102024
> 	* config/aarch64/aarch64.cc (aapcs_vfp_sub_candidate): Handle
> 	zero-sized bit-fields.  Detect cases where a warning may be needed.
> 	(aarch64_vfp_is_call_or_return_candidate): Emit a note if a
> 	zero-sized bit-field has caused parameter passing to change.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/aarch64/aapcs64/test_28.c: New test.
> ---
>  gcc/config/aarch64/aarch64.cc                 | 35 +++++++++++++++++--
>  .../gcc.target/aarch64/aapcs64/test_28.c      | 28 +++++++++++++++
>  2 files changed, 60 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/test_28.c
> 

> @@ -19711,8 +19734,10 @@ aarch64_vfp_is_call_or_return_candidate (machine_mode mode,
>  	      && ((alt = aapcs_vfp_sub_candidate (type, &new_mode, NULL))
>  		  != ag_count))
>  	    {
> -	      const char *url
> +	      const char *url10
>  		= CHANGES_ROOT_URL "gcc-10/changes.html#empty_base";
> +	      const char *url12
> +		= CHANGES_ROOT_URL "gcc-12/changes.html#empty_base";

Again, #zero_width_bitfields
Otherwise LGTM.

	Jakub

Re: [PATCH 1/2] arm: correctly handle zero-sized bit-fields in AAPCS [PR102024]

[Richard Earnshaw]

On 29/03/2022 17:32, Jakub Jelinek via Gcc-patches wrote:
> On Tue, Mar 29, 2022 at 04:32:10PM +0100, Richard Earnshaw wrote:
>>
>> On arm the AAPCS states that an HFA is determined by the 'shape' of
>> the object after layout has been completed, so anything that adds no
>> members and does not cause the layout to be modified should be ignored
>> for the purposes of determining which registers are used for parameter
>> passing.
>>
>> A zero-sized bit-field falls into this category.  This was not handled
>> correctly for C structs and in G++-11 only handled correctly because
>> such fields were eliminated early by the front end.
>>
>> gcc/ChangeLog:
>>
>> 	PR target/102024
>> 	* config/arm/arm.cc (aapcs_vfp_sub_candidate): Handle zero-sized
>> 	bit-fields.  Detect cases where a warning may be needed.
>> 	(aapcs_vfp_is_call_or_return_candidate): Emit a note if
>> 	a zero-sized bit-field has caused parameter passing to change.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	PR target/102024
>> 	* gcc.target/arm/aapcs/vfp26.c: New test.
>> ---
>>   gcc/config/arm/arm.cc                      | 35 ++++++++++++++++++++--
>>   gcc/testsuite/gcc.target/arm/aapcs/vfp26.c | 31 +++++++++++++++++++
>>   2 files changed, 63 insertions(+), 3 deletions(-)
>>   create mode 100644 gcc/testsuite/gcc.target/arm/aapcs/vfp26.c
>>
> 
>> diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc
>> index e062361b985..43b775f6918 100644
>> --- a/gcc/config/arm/arm.cc
>> +++ b/gcc/config/arm/arm.cc
>> @@ -6274,6 +6274,7 @@ aapcs_vfp_cum_init (CUMULATIVE_ARGS *pcum  ATTRIBUTE_UNUSED,
>>         a HFA or HVA.  */
>>   const unsigned int WARN_PSABI_EMPTY_CXX17_BASE = 1U << 0;
>>   const unsigned int WARN_PSABI_NO_UNIQUE_ADDRESS = 1U << 1;
>> +const unsigned int WARN_PSABI_ZERO_WIDTH_BITFIELD = 1U << 2;
>>   
>>   /* Walk down the type tree of TYPE counting consecutive base elements.
>>      If *MODEP is VOIDmode, then set it to the first valid floating point
>> @@ -6426,6 +6427,28 @@ aapcs_vfp_sub_candidate (const_tree type, machine_mode *modep,
>>   		    continue;
>>   		  }
>>   	      }
>> +	    /* A zero-width bitfield may affect layout in some
>> +	       circumstances, but adds no members.  The determination
>> +	       of whether or not a type is an HFA is performed after
>> +	       layout is complete, so if the type still looks like an
>> +	       HFA afterwards, it is still classed as one.  This is
>> +	       potentially an ABI break for the hard-float ABI.  */
>> +	    else if (DECL_BIT_FIELD (field)
>> +		     && integer_zerop (DECL_SIZE (field)))
>> +	      {
>> +		/* Prior to GCC-12 these fields were striped early,
>> +		   hiding them from the back-end entirely and
>> +		   resulting in the correct behaviour for argument
>> +		   passing.  Simulate that old behaviour without
>> +		   generating a warning.  */
>> +		if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))
>> +		  continue;
>> +		if (warn_psabi_flags)
>> +		  {
>> +		    *warn_psabi_flags |= WARN_PSABI_ZERO_WIDTH_BITFIELD;
>> +		    continue;
>> +		  }
>> +	      }
>>   
>>   	    sub_count = aapcs_vfp_sub_candidate (TREE_TYPE (field), modep,
>>   						 warn_psabi_flags);
>> @@ -6538,8 +6561,10 @@ aapcs_vfp_is_call_or_return_candidate (enum arm_pcs pcs_variant,
>>   	      && ((alt = aapcs_vfp_sub_candidate (type, &new_mode, NULL))
>>   		  != ag_count))
>>   	    {
>> -	      const char *url
>> +	      const char *url10
>>   		= CHANGES_ROOT_URL "gcc-10/changes.html#empty_base";
>> +	      const char *url12
>> +		= CHANGES_ROOT_URL "gcc-12/changes.html#empty_base";
> 
> This should be
> 		= CHANGES_ROOT_URL "gcc-12/changes.html#zero_width_bitfields";
> instead.
> 
> Otherwise LGTM.
> 
> 	Jakub
> 

Good catch. Thanks.  Updated and pushed both patches.

R.