[Bug c++/95726] New: ICE with aarch64 __Float32x4_t as template argument

[Bug c++/95726] New: ICE with aarch64 __Float32x4_t as template argument

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95726

            Bug ID: 95726
           Summary: ICE with aarch64 __Float32x4_t as template argument
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jakub at gcc dot gnu.org
  Target Milestone: ---

Following test ICEs on aarch64-linux:

// { dg-do compile }
// { dg-options "--param=hash-table-verification-limit=500 -O2" }

typedef __Float32x4_t float32x4_t;
typedef float V [[gnu::vector_size(4 * sizeof (float))]];

template <typename T>
int
foo ()
{
  return 0;
}

int
bar ()
{
  return foo <float32x4_t> () + foo <__Float32x4_t> () + foo <V> () + foo
<float [[gnu::vector_size(4 * sizeof (float))]]> ();
}

On 10.x branch I can reproduce the ICE even on a large unreduceable source
without that extra argument.

The function template is specialized twice, once with <vector(4) float> and
once with <__Float32x4_t>.
The former vector type is:
    elt:1 <vector_type 0x7fffea85cf18
        type <real_type 0x7fffea7742a0 float type_6 SF
            size <integer_cst 0x7fffea76e198 constant 32>
            unit-size <integer_cst 0x7fffea76e1b0 constant 4>
            align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffea7742a0 precision:32
            pointer_to_this <pointer_type 0x7fffea774888> reference_to_this
<reference_type 0x7fffe378d7e0>>
        type_6 V4SF size <integer_cst 0x7fffea754f90 128> unit-size
<integer_cst 0x7fffea754fa8 16>
        align:128 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffea85cf18 nunits:4>>
(and has TYPE_CANONICAL itself), while the other one is some vector type
created by the aarch64 backend:
    elt:1 <vector_type 0x7fffea85f000 __Float32x4_t
        type <real_type 0x7fffea7742a0 float type_6 SF
            size <integer_cst 0x7fffea76e198 constant 32>
            unit-size <integer_cst 0x7fffea76e1b0 constant 4>
            align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffea7742a0 precision:32
            pointer_to_this <pointer_type 0x7fffea774888> reference_to_this
<reference_type 0x7fffe378d7e0>>
        type_6 V4SF size <integer_cst 0x7fffea754f90 128> unit-size
<integer_cst 0x7fffea754fa8 16>
        align:128 warn_if_not_align:0 symtab:0 alias-set -1 structural-equality
nunits:4>>
(and has NULL TYPE_CANONICAL).
In iterative_hash_template_arg for these VECTOR_TYPEs we end up:
        default:
          if (tree canonical = TYPE_CANONICAL (arg))
            val = iterative_hash_object (TYPE_HASH (canonical), val);
and thus hash it differently between vector(4) float and __Float32x4_t.
But spec_hasher::equal calls comp_template_args for that and that in turn calls
same_type_p on those and
structural_comptypes is called because one of those lacks TYPE_CANONICAL, and
there it does:
    case VECTOR_TYPE:
      if (gnu_vector_type_p (t1) != gnu_vector_type_p (t2)
          || maybe_ne (TYPE_VECTOR_SUBPARTS (t1), TYPE_VECTOR_SUBPARTS (t2))
          || !same_type_p (TREE_TYPE (t1), TREE_TYPE (t2)))
        return false;
and considers the two the same.
Because it hashed differently, it is added twice into the hash table, and when
we are unlucky, the two hash values modulo the size of the hash table are
equal,
so we have two different specializations (instantiations?) for the same thing
and the check that retrieve_specialization returns the one we are considering
fails.

2020-06-17  Jakub Jelinek  <jakub@redhat.com>

        * pt.c (iterative_hash_template_arg): Handle VECTOR_TYPE.

--- gcc/cp/pt.c.jj      2020-06-17 18:48:53.506617982 +0200
+++ gcc/cp/pt.c 2020-06-17 19:34:28.551298600 +0200
@@ -1924,10 +1924,21 @@ iterative_hash_template_arg (tree arg, h
          }
          break;

-       case  DECLTYPE_TYPE:
+       case DECLTYPE_TYPE:
          val = iterative_hash_template_arg (DECLTYPE_TYPE_EXPR (arg), val);
          break;

+       case VECTOR_TYPE:
+         {
+           bool gnu_vec = gnu_vector_type_p (arg);
+           val = iterative_hash_object (gnu_vec, val);
+           val = iterative_hash_template_arg (TREE_TYPE (arg), val);
+           inchash::hash hstate (val);
+           hstate.add_poly_int (TYPE_VECTOR_SUBPARTS (arg));
+           val = hstate.end ();
+         }
+         break;
+
        default:
          if (tree canonical = TYPE_CANONICAL (arg))
            val = iterative_hash_object (TYPE_HASH (canonical), val);

fixes this for me, but I wonder if we don't need to handle at least
POINTER_TYPE and REFERENCE_TYPE too, because if a template argument
is not a vector type, but e.g. a pointer to vector or reference to vector, due
to the magic backend VECTOR_TYPEs with no TYPE_CANONICAL
pointers to them will also have structural equality and thus would hash
differently from pointers/references to the C++ FE created vector types.

The __Float32x4_t type is created in aarch64_init_simd_builtin_types using
build_distinct_type_copy and
explicitly requests structural equality:
          aarch64_simd_types[i].itype
            = build_distinct_type_copy
              (build_vector_type (eltype, GET_MODE_NUNITS (mode)));
          SET_TYPE_STRUCTURAL_EQUALITY (aarch64_simd_types[i].itype);

For POINTER_TYPE/REFERENCE_TYPE, we could do:
        case REFERENCE_TYPE:
          {
            bool rval = TYPE_REF_IS_RVALUE (arg);
            val = iterative_hash_object (rval, val);
          }
          /* FALLTHRU */
        case POINTER_TYPE:
          {
            machine_mode mode = TYPE_MODE (arg);
            val = iterative_hash_object (mode, val);
            val = iterative_hash_template_arg (TREE_TYPE (arg), val);
          }
          break;

Thoughts on this?

[Bug c++/95726] ICE with aarch64 __Float32x4_t as template argument

[jason at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95726

--- Comment #1 from Jason Merrill <jason at gcc dot gnu.org> ---
Does the aarch64 port expect __Float32x4_t type to be considered equivalent to
the GNU vector type or not?  If so, why use build_distinct_type_copy over
build_variant_type_copy?  If not, they might want to set TYPE_INDIVISIBLE_P so
that structural_comptypes treats them as different.

[Bug c++/95726] ICE with aarch64 __Float32x4_t as template argument

[jason at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95726

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2020-06-17

[Bug c++/95726] ICE with aarch64 __Float32x4_t as template argument

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95726

--- Comment #2 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Jason Merrill from comment #1)
> Does the aarch64 port expect __Float32x4_t type to be considered equivalent
> to the GNU vector type or not?  If so, why use build_distinct_type_copy over
> build_variant_type_copy?  If not, they might want to set TYPE_INDIVISIBLE_P
> so that structural_comptypes treats them as different.
They mangle differently, and e.g.:

void f(float32x4_t);
void f(V);

aren't ODR equivalent.  But a lot of code relies on the GNU vector
extensions being available for float32x4_t as well as V, and on V
and float32x4_t being mutually and implicitly interconvertible.

[Bug c++/95726] ICE with aarch64 __Float32x4_t as template argument

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95726

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to rsandifo@gcc.gnu.org from comment #2)
> They mangle differently, and e.g.:
> 
> void f(float32x4_t);
> void f(V);
> 
> aren't ODR equivalent.  But a lot of code relies on the GNU vector
> extensions being available for float32x4_t as well as V, and on V
> and float32x4_t being mutually and implicitly interconvertible.

But if they mangle differently, then structural_comptypes shouldn't treat them
as same types.

[Bug c++/95726] ICE with aarch64 __Float32x4_t as template argument

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95726

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rsandifo at gcc dot gnu.org

[Bug c++/95726] ICE with aarch64 __Float32x4_t as template argument

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95726

--- Comment #4 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> But if they mangle differently, then structural_comptypes shouldn't treat
> them as same types.
That certainly avoids the ICE, and makes GCC's behaviour consistent
with Clang for things like:

  typedef float vecf __attribute__((vector_size(16)));
  vecf x;
  float32x4_t &y = x;

Previously we accepted this, with the struct_comptypes change
we reject it (like Clang does).  But that might break existing
code, so I'm not sure it would be backportable.

I guess the question then is: what does TYPE_STRUCTURAL_EQUALITY_P
mean for VECTOR_TYPEs in the context of structural_comptypes?
And (if this is a different question) what case is that function's
VECTOR_TYPE handling for?  I.e. when do we want to return true for
a pair of VECTOR_TYPEs whose TYPE_MAIN_VARIANTs are different?

[Bug c++/95726] ICE with aarch64 __Float32x4_t as template argument

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95726

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Dunno, perhaps for backporting it could be done in template_args_equal instead?
Like I said, it would probably need to handle also POINTER/REFERENCE/ARRAY_TYPE
whose ultimate element type is VECTOR_TYPE possibly affected by this.
Dunno about aggregates, I'd hope we set TYPE_CANONICAL for most of them and
therefore shouldn't care about this.
If comptypes returns false for these, can one still implicitly convert them to
the other vector types?
Does 32-bit ARM have similar types too?
Anyway, your questions are more for Jason...

[Bug c++/95726] ICE with aarch64 __Float32x4_t as template argument

[jason at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95726

--- Comment #6 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to rsandifo@gcc.gnu.org from comment #4)
> (In reply to Jakub Jelinek from comment #3)
> > But if they mangle differently, then structural_comptypes shouldn't treat
> > them as same types.

Definitely.

> That certainly avoids the ICE, and makes GCC's behaviour consistent
> with Clang for things like:
> 
>   typedef float vecf __attribute__((vector_size(16)));
>   vecf x;
>   float32x4_t &y = x;
> 
> Previously we accepted this, with the struct_comptypes change
> we reject it (like Clang does).  But that might break existing
> code, so I'm not sure it would be backportable.

If necessary we could add a conversion between the pointer-to-vector types.

> I guess the question then is: what does TYPE_STRUCTURAL_EQUALITY_P
> mean for VECTOR_TYPEs in the context of structural_comptypes?

The same thing it means for any other type: setting TYPE_CANONICAL properly is
too hard, so use structural_comptypes.

> And (if this is a different question) what case is that function's
> VECTOR_TYPE handling for?  I.e. when do we want to return true for
> a pair of VECTOR_TYPEs whose TYPE_MAIN_VARIANTs are different?

We want to return true if they should be considered the same type.

Generally, TYPE_MAIN_VARIANT isn't sufficient for checking type identity, as it
only looks through variants of the outermost type:  If I have

typedef int myint;
typedef myint* myintptr;

taking TYPE_MAIN_VARIANT of myintptr gives myint*, not int*.  That's why
TYPE_CANONICAL was introduced, so we didn't need to do structural comparison
whenever we wanted to compare types.

[Bug c++/95726] ICE with aarch64 __Float32x4_t as template argument

[jason at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95726

--- Comment #7 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #5)
> Dunno, perhaps for backporting it could be done in template_args_equal
> instead?

For backporting we could treat them as different only if
comparing_specializations is set.

[Bug c++/95726] ICE with aarch64 __Float32x4_t as template argument

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95726

--- Comment #8 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Thanks for the pointers.  Putting the mangled name in a target-specific
attribute (like we do for SVE) seems to fix it.  It actually also keeps
the testcase in comment 4 “working”, which is unexpected (to me) and
seems a little dubious.  I guess it's good news for backporting though.

[Bug c++/95726] ICE with aarch64 __Float32x4_t as template argument

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95726

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Sandiford <rsandifo@gcc.gnu.org>:

https://gcc.gnu.org/g:31427b974ed7b7dd54e28fec595e731bf6eea8ba

commit r11-1741-g31427b974ed7b7dd54e28fec595e731bf6eea8ba
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Tue Jun 30 21:40:30 2020 +0100

    aarch64: Treat GNU and Advanced SIMD vectors as distinct [PR92789, PR95726]

    PR95726 is about template look-up for things like:

        foo<float vecf __attribute__((vector_size(16)))>
        foo<float32x4_t>

    The immediate cause of the problem is that the hash function usually
    returns different hashes for these types, yet the equality function
    thinks they are equal.  This then raises the question of how the types
    are supposed to be treated.

    I think the answer is that the GNU vector type should be treated as
    distinct from float32x4_t, not least because the two types mangle
    differently.  However, each type should implicitly convert to the other.

    This would mean that, as far as the PR is concerned, the hashing
    function is right to (sometimes) treat the types differently and
    the equality function is wrong to treat them as the same.

    The most obvious way to enforce the type difference is to use a
    target-specific type attribute.  That on its own is enough to fix
    the PR.  The difficulty is deciding whether the knock-on effects
    are acceptable.

    One obvious effect is that GCC then rejects:

        typedef float vecf __attribute__((vector_size(16)));
        vecf x;
        float32x4_t &z = x;

    on the basis that the types are no longer reference-compatible.
    I think that's again the correct behaviour, and consistent with
    current Clang.

    A trickier question is whether:

        vecf x;
        float32x4_t y;
        ⦠c ? x : y â¦

    should be valid, and if so, what its type should be [PR92789].
    As explained in the comment in the testcase, GCC and Clang both
    accepted this, but GCC chose the âthenâ type while Clang chose
    the âelseâ type.  This can lead to different mangling for (probably
    artificial) corner cases, as seen for âsel1â and âsel2â in the
    testcase.

    Adding the attribute makes GCC reject the conditional expression
    as ambiguous.  I think that too is the correct behaviour, for the
    reasons described in the testcase.  However, it does seem to have
    the potential to break existing code.

    It looks like aarch64_comp_type_attributes is missing cases for
    the SVE attributes, but I'll handle that in a separate patch.

    2020-06-30  Richard Sandiford  <richard.sandiford@arm.com>

    gcc/
            PR target/92789
            PR target/95726
            * config/aarch64/aarch64.c (aarch64_attribute_table): Add
            "Advanced SIMD type".
            (aarch64_comp_type_attributes): Check that the "Advanced SIMD type"
            attributes are equal.
            * config/aarch64/aarch64-builtins.c: Include stringpool.h and
            attribs.h.
            (aarch64_mangle_builtin_vector_type): Use the mangling recorded
            in the "Advanced SIMD type" attribute.
            (aarch64_init_simd_builtin_types): Add an "Advanced SIMD type"
            attribute to each Advanced SIMD type, using the mangled type
            as the attribute's single argument.

    gcc/testsuite/
            PR target/92789
            PR target/95726
            * g++.target/aarch64/pr95726.C: New test.

[Bug c++/95726] ICE with aarch64 __Float32x4_t as template argument

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95726

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|aarch64-linux               |aarch64*-*-* arm*-*-*

--- Comment #10 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Fixed on trunk for aarch64*-*-* only.  Still needs fixing for arm*-*-*
on trunk, and for both targets on branches.

[Bug c++/95726] ICE with aarch64 __Float32x4_t as template argument

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95726

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Sandiford <rsandifo@gcc.gnu.org>:

https://gcc.gnu.org/g:efe99cca78215e339ba79f0a900a896b4c0a3d36

commit r11-2022-gefe99cca78215e339ba79f0a900a896b4c0a3d36
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Fri Jul 10 19:06:45 2020 +0100

    arm: Treat GNU and Advanced SIMD vectors as distinct [PR92789, PR95726]

    This is an arm version of aarch64 patch r11-1741.  The approach
    is essentially identical, not much more than s/aarch64/arm/.

    To recap, PR95726 is about template look-up for things like:

        foo<float vecf __attribute__((vector_size(16)))>
        foo<float32x4_t>

    The immediate cause of the problem is that the hash function usually
    returns different hashes for these types, yet the equality function
    thinks they are equal.  This then raises the question of how the types
    are supposed to be treated.

    The answer we chose for AArch64 was that the GNU vector type should
    be treated as distinct from float32x4_t, but that each type should
    implicitly convert to the other.

    This would mean that, as far as the PR is concerned, the hashing
    function is right to (sometimes) treat the types differently and
    the equality function is wrong to treat them as the same.

    The most obvious way to enforce the type difference is to use a
    target-specific type attribute.  That on its own is enough to fix
    the PR.  The difficulty is deciding whether the knock-on effects
    are acceptable.

    One obvious effect is that GCC then rejects:

        typedef float vecf __attribute__((vector_size(16)));
        vecf x;
        float32x4_t &z = x;

    on the basis that the types are no longer reference-compatible.
    For AArch64 we took the approach that this was the correct behaviour.
    It is also consistent with current Clang.

    A trickier question is whether:

        vecf x;
        float32x4_t y;
        ⦠c ? x : y â¦

    should be valid, and if so, what its type should be [PR92789].
    As explained in the comment in the testcase, GCC and Clang both
    accepted this, but GCC chose the âthenâ type while Clang chose
    the âelseâ type.  This can lead to different mangling for (probably
    artificial) corner cases, as seen for âsel1â and âsel2â in the
    testcase.

    Adding the attribute makes GCC reject the conditional expression
    as ambiguous.  For AArch64 we took the approach that this too is
    the correct behaviour, for the reasons described in the testcase.
    However, it does seem to have the potential to break existing code.

    gcc/
            PR target/92789
            PR target/95726
            * config/arm/arm.c (arm_attribute_table): Add
            "Advanced SIMD type".
            (arm_comp_type_attributes): Check that the "Advanced SIMD type"
            attributes are equal.
            * config/arm/arm-builtins.c: Include stringpool.h and
            attribs.h.
            (arm_mangle_builtin_vector_type): Use the mangling recorded
            in the "Advanced SIMD type" attribute.
            (arm_init_simd_builtin_types): Add an "Advanced SIMD type"
            attribute to each Advanced SIMD type, using the mangled type
            as the attribute's single argument.

    gcc/testsuite/
            PR target/92789
            PR target/95726
            * g++.target/arm/pr95726.C: New test.

[Bug c++/95726] ICE with aarch64 __Float32x4_t as template argument

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95726

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Richard Sandiford
<rsandifo@gcc.gnu.org>:

https://gcc.gnu.org/g:932e9140d3268cf2033c1c3e93219541c53fcd29

commit r10-8501-g932e9140d3268cf2033c1c3e93219541c53fcd29
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Wed Jul 15 11:58:04 2020 +0100

    c++: Treat GNU and Advanced SIMD vectors as distinct [PR95726]

    This is a release branch version of
    r11-1741-g:31427b974ed7b7dd54e28fec595e731bf6eea8ba and
    r11-2022-g:efe99cca78215e339ba79f0a900a896b4c0a3d36.

    The trunk versions of the patch made GNU and Advanced SIMD vectors
    distinct (but inter-convertible) in all cases.  However, the
    traditional behaviour is that the types are distinct in template
    arguments but not otherwise.

    Following a suggestion from Jason, this patch puts the check
    for different vector types under comparing_specializations.
    In order to keep the backport as simple as possible, the patch
    hard-codes the name of the attribute in the frontend rather than
    adding a new branch-only target hook.

    I didn't find a test that tripped the assert on the branch,
    even with the --param in the PR, so instead I tested this by
    forcing the hash function to only hash the tree code.  That made
    the static assertion in the test fail without the patch but pass
    with it.

    This means that the tests pass for unmodified sources even
    without the patch (unless you're very unlucky).

    gcc/
            PR target/95726
            * config/aarch64/aarch64.c (aarch64_attribute_table): Add
            "Advanced SIMD type".
            * config/aarch64/aarch64-builtins.c: Include stringpool.h and
            attribs.h.
            (aarch64_init_simd_builtin_types): Add an "Advanced SIMD type"
            attribute to each Advanced SIMD type.
            * config/arm/arm.c (arm_attribute_table): Add "Advanced SIMD type".
            * config/arm/arm-builtins.c: Include stringpool.h and attribs.h.
            (arm_init_simd_builtin_types): Add an "Advanced SIMD type"
            attribute to each Advanced SIMD type.

    gcc/cp/
            PR target/95726
            * typeck.c (structural_comptypes): When comparing template
            specializations, differentiate between vectors that have and
            do not have an "Advanced SIMD type" attribute.

    gcc/testsuite/
            PR target/95726
            * g++.target/aarch64/pr95726.C: New test.
            * g++.target/arm/pr95726.C: Likewise.

[Bug c++/95726] ICE with aarch64 __Float32x4_t as template argument

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95726

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Should be fixed now.  Note, PR96377 has been a regression introduced by this
fix.