On Fri, Feb 04, 2022 at 04:42:41PM -0500, Jason Merrill wrote:
> > @@ -20,9 +20,16 @@ along with GCC; see the file COPYING3.
> >   #ifndef GCC_FOLD_CONST_H
> >   #define GCC_FOLD_CONST_H
> > -/* Non-zero if we are folding constants inside an initializer; zero
> > -   otherwise.  */
> > +/* Nonzero if we are folding constants inside an initializer or a C++
> > +   manifestly-constant-evaluated context; zero otherwise.
> > +   Should be used when folding in initializer enables additional
> > +   optimizations.  */
> >   extern int folding_initializer;
> > +/* Nonzer of we are folding C++ manifestly-constant-evaluated context; zero
> 
> Still need to fix this typo.

Sorry, finally now fixed in my copy.

> > +   otherwise.
> > +   Should be used when certain constructs shouldn't be optimized
> > +   during folding in that context.  */
> > +bool folding_cxx_constexpr = false;
> > +
> >   /* The following constants represent a bit based encoding of GCC's
> >      comparison operators.  This encoding simplifies transformations
> >      on relational comparison operators, such as AND and OR.  */
> > @@ -16628,41 +16636,55 @@ address_compare (tree_code code, tree ty
> 
> Incidentally, the function comment needs to document TYPE.

Will do.

> So at this point equal can be 0 or 2, the latter because either the offset
> is out of bounds, or because we're comparing offset 0 to one-past-the-end.

Yes.

> In the out-of-bounds case, we're into undefined behavior, so I'd be inclined
> to return 2 immediately rather than continue, so the code below only needs
> to worry about possibly overlapping/contiguous objects.

You mean for folding_cxx_constexpr ?  The code does that basically, with one
exception, the folding_initializer FUNCTION_DECL cmp FUNCTION_DECL case.
We don't track sizes of functions, so the size of 1 is just a hack to
pretend functions don't have zero size.  Some functions can have zero size
if they contain just __builtin_unreachable, but it is very rare.
But I guess I could move that
  if (folding_initializer
      && TREE_CODE (base0) == FUNCTION_DECL
      && TREE_CODE (base1) == FUNCTION_DECL)
    return 0;
above the size checking block and then indeed right after that do
  if (folding_cxx_constexpr && equal)
    return equal;
with a comment.

> In the code below, in !constexpr mode we decide to return 0 even though
> equal == 2 in three cases which need more commentary, either together or
> separately:
> 
> 1) One is a string and the other a decl.  Do we know that we can't layout a
> string and a global variable next to each other?  This overlaps a lot
> with...
> 
> 2) We're comparing as pointers (rather than integers), so we return unequal
> even if they could be equal in practice if the objects are contiguous.  The
> comment says this but still needs a rationale; it doesn't seem useful to me
> for the limited cases that could reach here with equal == 2.

For the pointer comparisons we just exploit the undefined behavior and
pretend they can't be adjacent even if they actually sometimes can be,
and we've been doing that intentionally for years.
If one does (uintptr_t) &x == (uintptr_t) &y, we try to be more
conservative.

> 3) We're comparing a local variable and a global, so they really can't be
> equal unless the offset is far out of bounds.  This is currently last; we
> might move it first and treat strings like globals?

For the automatic vs. global or strings, it is very unlikely they'd be
adjacent, with typical memory layouts there couldn't be any heap and stack
would need to grow until it reaches end of data or bss section on a page
boundary.  Strings perhaps could be adjacent in .rodata, but again it is
fairly rare.
And sure, I can try to improve comments.

        Jakub

Reply via email to