[cairo] [PATCH 50/51] core: changed retval of _cairo_composite_rectangles_init_for_mask() to cairo_bool_t

[Enrico Weigelt, metux IT consult]

On 21.12.2015 17:55, Chris Wilson wrote:

>> The current implementation explicitly assigns the return value to a
>> local (likely on-stack) variable an then checks it. At least we should
>> directly compare the it, w/o assignment, IMHO.
> 
> Which the compiler eats, it should not hit stack until later (or at all,
> since it will be outside of its live range).

hmm, you mean in code like this

>>> status = foo(..)
>>> if (status)
>>>  return status;
>>> ...
>>> status = bar(..)
>>> if (status)
>>>  return status
>>> ...
>>> return status;

The first assignment wont actually store it on stack ?
That would mean somethink like this:

>>> push ...     // push the parameters
>>> call foo
>>> // retval now is in eax
>>> test eax
>>> cjmp label1
>>> undo stack;
>>> ret
>>> label1:
>>> // we dont store eax into status as we know it will
>>> // be overwritten anyways
>>> [...]
>>> push ...
>>> call bar
>>> [...]

Okay, pretty clear so far. But what about the second call (to bar())
and the final return ?

I'd guess the compiler can't see that on the final return, status can
only be zero. And as eax holds the return value after a call, it will
be overwritten by all subsequent calls, so it needs to store at least
the status variable after the second call. IMHO, that should be elimited
by making the final return constant (instead of status variable).


--mtx

--
Enrico Weigelt,
metux IT consulting
+49-151-27565287