[PATCH] tree-optimization/71831 - __builtin_object_size poor results with no optimization

[Martin Sebor]

On 08/20/2016 01:26 AM, Jakub Jelinek wrote:
> On Fri, Aug 19, 2016 at 04:30:47PM -0600, Martin Sebor wrote:
>> The patch looks bigger than it actually is because:
>>
>> 1) It modifies the return type of the function to bool rather than
>>     unsigned HOST_WIDE_INT representing the object size (this was
>>     necessary to avoid having its callers misinterpret zero as
>>     unknown when it means zero bytes).
>
> Can you explain why do you need this?  I don't understand why do you need to
> differentiate between unknown and maximum (or minimum for modes 2 and 3 that
> nobody actually uses in real-world), the builtin after all returns the same
> value for both.  If you want to know if the compiler knows the size
> precisely, you can request both mode 0 (or 1) and 2 (or 3) and compare, if
> the values are the same, it is the exact size, if there is a range, then you
> have minimum and maximum (and, if minimum is 0, but maximum non-zero, you
> really don't know minimum, if maximum is -1, then you really don't know the
> maximum (no object should be better that big).  For the return value, I
> don't see how you could reliably differentiate between the two even if it
> made for whatever strange reason sense - for SSA_NAMEs etc. you have just
> recorded the sizes, not also a flag whether it is unknown or known.

The change makes it possible to fold into constants even at -O0 type
2 and 3 calls to the built-in with zero-sized objects.

fold_builtin_object_size repeatedly calls compute_builtin_object_size
to compute the same result (zero) only to interpret it as a failure
and try over and over, never succeeding.  The built-in call is
ultimately expanded into a zero but that's too late to eliminate code
that depends on it. For example, the following emits the call to abort
that's never executed.

   char a[2];

   void f (void)
   {
     if (__builtin_object_size (a + 2, 2) != 0)
       __builtin_abort ();
   }

With the change, compute_builtin_object_size is called just once and
the call to abort is not emitted.

The initial version of the test verified this folding but adding code
to work around the built-ins various idiosyncrasies inadvertently wound
up removing it.  The attached patch adds a new test to exercise this
folding.

>
>> 2) As a result of a small change to the conditional that controls
>>     the main algorithm of the compute_builtin_object_size function
>>     it changes the depth of its indentation (without actually
>>     changing any of the code there).
>
> If you've done lots of redindentation, then additionally diff -upb would be
> appreciated.

Sure.  The attached diff was created without regard to whitespace.

Martin


-------------- next part --------------
A non-text attachment was scrubbed...
Name: gcc-71831.w.diff
Type: text/x-patch
Size: 24007 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20160821/a66772d4/attachment.bin>