Re: [PATCH] c++, v2: Further address_compare fixes [PR89074]

[Jason Merrill <jason@redhat.com>]

On 2/3/22 15:33, Jakub Jelinek wrote:
> On Thu, Feb 03, 2022 at 03:07:03PM -0500, Jason Merrill wrote:
>>> --- gcc/fold-const.h.jj	2022-02-01 20:10:51.235856007 +0100
>>> +++ gcc/fold-const.h	2022-02-03 15:02:02.700228631 +0100
>>> -/* 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
>>
>> "Nonzero if"
> 
> Oops, thanks for catching it.
> 
>>> +  if (!folding_cxx_constexpr
>>> +      && ((DECL_P (base0) && TREE_CODE (base1) == STRING_CST)
>>> +	  || (TREE_CODE (base0) == STRING_CST && DECL_P (base1))
>>> +	  || (TREE_CODE (base0) == STRING_CST
>>> +	      && TREE_CODE (base1) == STRING_CST
>>> +	      && ioff0 >= 0 && ioff1 >= 0
>>> +	      && ioff0 < TREE_STRING_LENGTH (base0)
>>> +	      && ioff1 < TREE_STRING_LENGTH (base1)
>>> +	      /* This is a too conservative test that the STRING_CSTs
>>> +		 will not end up being string-merged.  */
>>> +	      && strncmp (TREE_STRING_POINTER (base0) + ioff0,
>>> +			  TREE_STRING_POINTER (base1) + ioff1,
>>> +			  MIN (TREE_STRING_LENGTH (base0) - ioff0,
>>> +			       TREE_STRING_LENGTH (base1) - ioff1)) != 0)))
>>>        ;
>>> -  else if (!DECL_P (base0) || !DECL_P (base1))
>>> +  /* Punt on non-zero offsets from functions.  */
>>> +  else if ((TREE_CODE (base0) == FUNCTION_DECL && ioff0)
>>> +	   || (TREE_CODE (base1) == FUNCTION_DECL && ioff1))
>>> +    return 2;
>>> +  else if ((!DECL_P (base0)
>>> +  	    && (!folding_cxx_constexpr || TREE_CODE (base0) != STRING_CST))
>>> +	   || (!DECL_P (base1)
>>> +	       && (!folding_cxx_constexpr || TREE_CODE (base1) != STRING_CST)))
>>
>> I think it would be clearer to leave the !DECL_P case alone and add
>>
>> /* In C++ it is unspecified, and so non-constant, whether two
>>     equivalent strings have the same address.  */
>> else if (folding_cxx_constexpr
>>           && (TREE_CODE (base0) == STRING_CST
>>               || TREE_CODE (base1) == STRING_CST)
> 
> The point was to let the first if handle for
> !folding_cxx_constexpr the cases with STRING_CST
> as one or both operands and if that falls through, return 2.

Ah, I see.  And then for folding_cxx_constexpr you have your new code 
toward the bottom of the function that can say they're unequal in some 
cases.  Can you combine the STRING_CST handling for both values of 
folding_cxx_constexpr instead of having them so far apart?

Jason