[Bug tree-optimization/107833] New: wrong code at -Os and above on x86_64-linux-gnu

[Bug tree-optimization/107833] New: wrong code at -Os and above on x86_64-linux-gnu

[zhendong.su at inf dot ethz.ch]

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

            Bug ID: 107833
           Summary: wrong code at -Os and above on x86_64-linux-gnu
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zhendong.su at inf dot ethz.ch
  Target Milestone: ---

Compiler Explorer: https://godbolt.org/z/Tc8vbearG

It appears to be a regression from 11.3.

[561] % gcctk -v
Using built-in specs.
COLLECT_GCC=gcctk
COLLECT_LTO_WRAPPER=/local/suz-local/software/local/gcc-trunk/libexec/gcc/x86_64-pc-linux-gnu/13.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../gcc-trunk/configure --disable-bootstrap
--enable-checking=yes --prefix=/local/suz-local/software/local/gcc-trunk
--enable-sanitizers --enable-languages=c,c++ --disable-werror --enable-multilib
--with-system-zlib
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 13.0.0 20221123 (experimental) [master r13-4262-g1cac00d0138] (GCC) 
[562] % 
[562] % gcctk -O1 small.c; ./a.out
[563] % 
[563] % gcctk -Os small.c
[564] % ./a.out
Segmentation fault
[565] % 
[565] % cat small.c
int printf(const char *, ...);
int a, b[1] = {0}, c, *d = b, e, *f, g;
int main() {
  int h = 0;
  for (; a < 2; a++) {
    int i;
    for (g = 0; g < 2; g++)
      if (a < h) {
        e = i % 2;
        c = *f;
      }
    for (h = 0; h < 3; h++) {
      if (d)
        break;
      i--;
      printf("0");
    }
  }
  return 0;
}

[Bug tree-optimization/107833] wrong code at -Os and above on x86_64-linux-gnu since r12-5138-ge82c382971664d6f

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-11-23
   Target Milestone|---                         |12.3
            Summary|wrong code at -Os and above |wrong code at -Os and above
                   |on x86_64-linux-gnu         |on x86_64-linux-gnu since
                   |                            |r12-5138-ge82c382971664d6f
                 CC|                            |aldyh at gcc dot gnu.org,
                   |                            |marxin at gcc dot gnu.org
     Ever confirmed|0                           |1
           Keywords|                            |wrong-code
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r12-5138-ge82c382971664d6f.

[Bug tree-optimization/107833] wrong code at -Os and above on x86_64-linux-gnu since r12-5138-ge82c382971664d6f

[aldyh at gcc dot gnu.org]

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

--- Comment #2 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
(In reply to Zhendong Su from comment #0)
> Compiler Explorer: https://godbolt.org/z/Tc8vbearG
> 
> It appears to be a regression from 11.3.
> 
> [561] % gcctk -v
> Using built-in specs.
> COLLECT_GCC=gcctk
> COLLECT_LTO_WRAPPER=/local/suz-local/software/local/gcc-trunk/libexec/gcc/
> x86_64-pc-linux-gnu/13.0.0/lto-wrapper
> Target: x86_64-pc-linux-gnu
> Configured with: ../gcc-trunk/configure --disable-bootstrap
> --enable-checking=yes --prefix=/local/suz-local/software/local/gcc-trunk
> --enable-sanitizers --enable-languages=c,c++ --disable-werror
> --enable-multilib --with-system-zlib
> Thread model: posix
> Supported LTO compression algorithms: zlib
> gcc version 13.0.0 20221123 (experimental) [master r13-4262-g1cac00d0138]
> (GCC) 
> [562] % 
> [562] % gcctk -O1 small.c; ./a.out
> [563] % 
> [563] % gcctk -Os small.c
> [564] % ./a.out
> Segmentation fault
> [565] % 
> [565] % cat small.c
> int printf(const char *, ...);
> int a, b[1] = {0}, c, *d = b, e, *f, g;
> int main() {
>   int h = 0;
>   for (; a < 2; a++) {
>     int i;
>     for (g = 0; g < 2; g++)
>       if (a < h) {
>         e = i % 2;

Isn't there an uninitialized read from "i" here?  At least on the second time
through the outer loop, if (a < h) is true since 1 < 0.

>         c = *f;
>       }
>     for (h = 0; h < 3; h++) {
>       if (d)
>         break;

d is nonzero so h will always be 0 in this function.

>       i--;
>       printf("0");
>     }
>   }
>   return 0;
> }

Unless I'm missing something....

[Bug tree-optimization/107833] wrong code at -Os and above on x86_64-linux-gnu since r12-5138-ge82c382971664d6f

[marxin at gcc dot gnu.org]

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

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
> Isn't there an uninitialized read from "i" here?

Yes ...

>  At least on the second
> time through the outer loop, if (a < h) is true since 1 < 0.
> 
> >         c = *f;
> >       }
> >     for (h = 0; h < 3; h++) {
> >       if (d)
> >         break;
> 
> d is nonzero so h will always be 0 in this function.

... but as you correctly said, it's never executed as 'a < h' is always false.

> 
> >       i--;
> >       printf("0");
> >     }
> >   }
> >   return 0;
> > }
> 
> Unless I'm missing something....

[Bug tree-optimization/107833] wrong code at -Os and above on x86_64-linux-gnu since r12-5138-ge82c382971664d6f

[aldyh at gcc dot gnu.org]

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

--- Comment #4 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
(In reply to Martin Liška from comment #3)
> > Isn't there an uninitialized read from "i" here?
> 
> Yes ...
> 
> >  At least on the second
> > time through the outer loop, if (a < h) is true since 1 < 0.
> > 
> > >         c = *f;
> > >       }
> > >     for (h = 0; h < 3; h++) {
> > >       if (d)
> > >         break;
> > 
> > d is nonzero so h will always be 0 in this function.
> 
> ... but as you correctly said, it's never executed as 'a < h' is always
> false.

Huh... me thinking 1 < 0 is true is a clear sign I need to go on vacation.

/me hides in shame

[Bug tree-optimization/107833] wrong code at -Os and above on x86_64-linux-gnu since r12-5138-ge82c382971664d6f

[marxin at gcc dot gnu.org]

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

--- Comment #5 from Martin Liška <marxin at gcc dot gnu.org> ---
> /me hides in shame

:DD

[Bug tree-optimization/107833] [12/13 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5138-ge82c382971664d6f

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|wrong code at -Os and above |[12/13 Regression] wrong
                   |on x86_64-linux-gnu since   |code at -Os and above on
                   |r12-5138-ge82c382971664d6f  |x86_64-linux-gnu since
                   |                            |r12-5138-ge82c382971664d6f
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
might be a latent issue as well

[Bug tree-optimization/107833] [12/13 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5138-ge82c382971664d6f

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
printf isn't needed, -O2:
int a, b[1] = { 0 }, c, *d = b, e, *f, g;

__attribute__((noipa)) int
foo (const char *x)
{
  (void) x;
  return 0;
}

int
main ()
{
  for (int h = 0; a < 2; a++)
    {
      int i;
      for (g = 0; g < 2; g++)
        if (a < h)
          {
            e = i % 2;
            c = *f;
          }
      for (h = 0; h < 3; h++)
        {
          if (d)
            break;
          i--;
          foo ("0");
        }
    }
  return 0;
}

is miscompiled as well.

[Bug tree-optimization/107833] [12/13 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5138-ge82c382971664d6f

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think this is CCP's fault rather than VRP.
Everything I've looked at in the vrp2 dump looks right to me.
In the outer loop, h/i is
  # RANGE [irange] int [0, 3] NONZERO 0x3
  # h_41 = PHI <h_21(8), 0(2)>
  # i_44 = PHI <i_43(8), i_19(D)(2)>
and the second inner loop has
  _13 = (unsigned int) i_44; // This is before the second inner loop
...
  <bb 6> [local count: 304150043]:
  # i_42 = PHI <i_23(7), i_44(5)>
  _50 = (unsigned int) i_42;
  _17 = -_50;
  _34 = _13 - _50;
  # RANGE [irange] int [0, 2] NONZERO 0x3
  h_40 = (int) _34;
where
  i_23 = i_42 + -1;
and
  # h_21 = PHI <h_40(6), 3(7)>
Now, sure, i is uninitialized (the i_19(D) in PHI of the outer loop),
but as has been discussed in the first loop a < h isn't true and so we don't
use
i there, and in the second loop d isn't NULL and so we don't i-- there either,
but
due to the function call we must reread the d value after each iteration.
Which is the reason for the h_40 = (int) ((unsigned) i_44 - i_42) computation
to get h
when d is non-NULL in some iteration - if d is always NULL, it would be 3 in
the PHI.
But ccp4 goes wild on this,
Visiting PHI node: i_44 = PHI <i_43(8), i_19(D)(2)>
        Argument #0 (8 -> 3 not executable)
        Argument #1 (2 -> 3 executable)
        i_19(D) Value: UNDEFINED

    PHI node value: UNDEFINED

Lattice value changed to UNDEFINED.  Adding SSA edges to worklist.
...
Visiting statement:
_13 = (unsigned int) i_44;
which is likely UNDEFINED
Lattice value changed to UNDEFINED.  Adding SSA edges to worklist.
...
Visiting statement:
_50 = (unsigned int) i_42;
which is likely UNDEFINED
Lattice value changed to UNDEFINED.  Adding SSA edges to worklist.
...
Visiting statement:
# RANGE [irange] int [0, 2] NONZERO 0x3
h_40 = (int) _34;
which is likely UNDEFINED
Lattice value changed to UNDEFINED.  Adding SSA edges to worklist.
etc., everything based on i is UNDEFINED.  So, either our UNDEFINED handlingin
CCP
is incorrect, or we need to avoid creating that h_40 = (int) ((unsigned) i_44 -
i_42)
stuff in IVOPTS if it is based on uninitialized (but then, can't we get an
uninitialized value propagated to it only after IVOPTS?).

[Bug tree-optimization/107833] [12/13 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5138-ge82c382971664d6f

[jakub at gcc dot gnu.org]

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Wasn't this discussed in the past in other PRs?  Whether uninitialized vars
mean anything or anything but same value each time a SSA_NAME initialized to it
is used?
Because as this testcase shows, not all uses of uninitialized SSA_NAMEs are
actually UB, at least their use in PHIs must be ok, and if IVOPTS or other
optimizations can construct some expressions computing a difference between
uninitialized loop iterator's PHI header between several iterations (e.g. to
compute final value), it better would be the second definition rather than the
first.

[Bug tree-optimization/107833] [12/13 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5138-ge82c382971664d6f

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=100810

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #9)
> Wasn't this discussed in the past in other PRs?  Whether uninitialized vars
> mean anything or anything but same value each time a SSA_NAME initialized to
> it is used?

I think you are talking about PR 100810 and the discussions around the patch
that fixed that.

[Bug tree-optimization/107833] [12/13 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5138-ge82c382971664d6f

[rguenth at gcc dot gnu.org]

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

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #10)
> (In reply to Jakub Jelinek from comment #9)
> > Wasn't this discussed in the past in other PRs?  Whether uninitialized vars
> > mean anything or anything but same value each time a SSA_NAME initialized to
> > it is used?
> 
> I think you are talking about PR 100810 and the discussions around the patch
> that fixed that.

Yep, it looks exactly like such a case - IVOPTs is replacing 'h' in the
second inner loop with an expression based on 'i' and places the computation
of 'h' before the early exit (because that's where the original 'h' IV PHI
is defined).  I think the placement doesn't make a difference but replacing
the loop exit test does?

Anyway, GCC doesn't ensure stability of uninitialized values which means
any expressions derived become indeterminate, i_1(D) - i_1(D) isn't
guaranteed to be zero if you obfuscate that a bit.  That puts the burden
on compiler passes to never introduce new intermediate values since
whether any of the expression participating values are not initialized
might only become visible later.  OK, maybe that's too restrictive, but
I certainly warned that the fix in PR100810 is going to be whack-a-mole
playing.

Here we have

marking _19 as maybe-undef
marking _44 as maybe-undef because of _19

but then we have

  <bb 3> [local count: 118111601]:
  # a.6_39 = PHI <_8(13), a.6_32(11)>
  # h_41 = PHI <h_21(13), 0(11)>
  # i_44 = PHI <i_43(13), i_19(D)(11)>
  g = 0; 
  _1 = i_44 % 2;
  f.1_2 = f; 
  if (a.6_39 < h_41)

note how the use of i was hoisted which makes the

          /* Look for any uses of the maybe-unused SSA_NAME that
             dominates the block that reaches the incoming block
             corresponding to the PHI arg in which it is mentioned.
             That means we can assume the SSA_NAME is defined in that
             path, so we only mark a PHI result as maybe-undef if we
             find an unused reaching SSA_NAME.  */
          int idx = phi_arg_index_from_use (use_p);
          basic_block bb = gimple_phi_arg_edge (phi, idx)->src;
          if (ssa_name_any_use_dominates_bb_p (var, bb))
            continue;

logic fail.  It's invariant motion that moves this use (I think we have
a duplicate issue with regarding to -Wuninitialized for this kind of
transform):

_1 = i_44 % 2;
  invariant up to level 2, cost 20.
...
Moving statement
_1 = i_44 % 2;
(cost 20) out of loop 2.

We could try to use the same mark_ssa_maybe_undefs in invariant motion
but it might not be a trivial exercise.  I also question sustainability
of how we extend exploitation of undefinedness in GCC ... :/

[Bug tree-optimization/107833] [12/13 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5138-ge82c382971664d6f

[jakub at gcc dot gnu.org]

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, do we whack-a-mole in this case some more and try harder in
mark_ssa_maybe_undefs
by basically repeating the CCP4 UNDEFINED propagation, or invent some new stmt
or marking that IVOPTS could use before the loop to turn possibly undefined
into undefined but always the same random pattern within the same invocation
(kind of like asm ("" : "+g" (undef));), or something else (like after IVOPTS
stop the UNDEFINED propagation on loop header PHIs or something similar)?

[Bug tree-optimization/107833] [12/13 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5138-ge82c382971664d6f

[rguenther at suse dot de]

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

--- Comment #13 from rguenther at suse dot de <rguenther at suse dot de> ---
On Fri, 2 Dec 2022, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107833
> 
> --- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> So, do we whack-a-mole in this case some more and try harder in
> mark_ssa_maybe_undefs
> by basically repeating the CCP4 UNDEFINED propagation, or invent some new stmt
> or marking that IVOPTS could use before the loop to turn possibly undefined
> into undefined but always the same random pattern within the same invocation
> (kind of like asm ("" : "+g" (undef));), or something else (like after IVOPTS
> stop the UNDEFINED propagation on loop header PHIs or something similar)?

Given the implementation of mark_ssa_maybe_undefs we can either blame
(and fix) invariant motion (-fno-tree-loop-im fixes the testcase as well)
or remove the ssa_name_any_use_dominates_bb_p check in this function,
marking more SSA names as possibly undefined.

We can also choose to ignore the issue until we run into this in the
real world or try to design something better here (I keep being pointed
at LLVM here but I don't see them solving the issue either).

I will see whether I can make invariant motion not hoist undefined
names.

[Bug tree-optimization/107833] [12/13 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5138-ge82c382971664d6f

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

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

--- Comment #14 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 54005
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54005&action=edit
patch for LIM

I'm testing this patch to limit invariant motion which also fixes PR107839.

[Bug tree-optimization/107833] [12/13 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5138-ge82c382971664d6f

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:44c8402d35160515b3c09fd2bc239587e0c32a2b

commit r13-4491-g44c8402d35160515b3c09fd2bc239587e0c32a2b
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Dec 2 14:52:20 2022 +0100

    tree-optimization/107833 - invariant motion of uninit uses

    The following fixes a wrong-code bug caused by loop invariant motion
    hoisting an expression using an uninitialized value outside of its
    controlling condition causing IVOPTs to use that to rewrite a defined
    value.  PR107839 is a similar case involving a bogus uninit diagnostic.

            PR tree-optimization/107833
            PR tree-optimization/107839
            * cfghooks.cc: Include tree.h.
            * tree-ssa-loop-im.cc (movement_possibility): Wrap and
            make stmts using any ssa_name_maybe_undef_p operand
            to preserve execution.
            (loop_invariant_motion_in_fun): Call mark_ssa_maybe_undefs
            to init maybe-undefined status.
            * tree-ssa-loop-ivopts.cc (ssa_name_maybe_undef_p,
            ssa_name_set_maybe_undef, ssa_name_any_use_dominates_bb_p,
            mark_ssa_maybe_undefs): Move ...
            * tree-ssa.cc: ... here.
            * tree-ssa.h (ssa_name_any_use_dominates_bb_p,
            mark_ssa_maybe_undefs): Declare.
            (ssa_name_maybe_undef_p, ssa_name_set_maybe_undef): Define.

            * gcc.dg/torture/pr107833.c: New testcase.
            * gcc.dg/uninit-pr107839.c: Likewise.

[Bug tree-optimization/107833] [12 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5138-ge82c382971664d6f

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:812847a9d12c0b65695cbe1a23959b69a7e62355

commit r12-8977-g812847a9d12c0b65695cbe1a23959b69a7e62355
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Dec 2 14:52:20 2022 +0100

    tree-optimization/107833 - invariant motion of uninit uses

    The following fixes a wrong-code bug caused by loop invariant motion
    hoisting an expression using an uninitialized value outside of its
    controlling condition causing IVOPTs to use that to rewrite a defined
    value.  PR107839 is a similar case involving a bogus uninit diagnostic.

            PR tree-optimization/107833
            PR tree-optimization/107839
            * cfghooks.cc: Include tree.h.
            * tree-ssa-loop-im.cc (movement_possibility): Wrap and
            make stmts using any ssa_name_maybe_undef_p operand
            to preserve execution.
            (loop_invariant_motion_in_fun): Call mark_ssa_maybe_undefs
            to init maybe-undefined status.
            * tree-ssa-loop-ivopts.cc (ssa_name_maybe_undef_p,
            ssa_name_set_maybe_undef, ssa_name_any_use_dominates_bb_p,
            mark_ssa_maybe_undefs): Move ...
            * tree-ssa.cc: ... here.
            * tree-ssa.h (ssa_name_any_use_dominates_bb_p,
            mark_ssa_maybe_undefs): Declare.
            (ssa_name_maybe_undef_p, ssa_name_set_maybe_undef): Define.

            * gcc.dg/torture/pr107833.c: New testcase.
            * gcc.dg/uninit-pr107839.c: Likewise.

    (cherry picked from commit 44c8402d35160515b3c09fd2bc239587e0c32a2b)

[Bug tree-optimization/107833] [12 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5138-ge82c382971664d6f

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |12.2.1
             Status|ASSIGNED                    |RESOLVED
      Known to fail|                            |12.2.0
         Resolution|---                         |FIXED

--- Comment #17 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.