From: Alexei Starovoitov
> On Wed, Jun 11, 2014 at 8:09 AM, Daniel Borkmann <dborkman@xxxxxxxxxx> wrote:
> > On 06/11/2014 04:55 PM, David Laight wrote:
> >>
> >> From: Daniel Borkmann
...
> >>> --- a/net/sctp/associola.c
> >>> +++ b/net/sctp/associola.c
> >>> @@ -1591,7 +1591,7 @@ int sctp_assoc_lookup_laddr(struct sctp_association
> >>> *asoc,
> >>> /* Set an association id for a given association */
> >>> int sctp_assoc_set_id(struct sctp_association *asoc, gfp_t gfp)
> >>> {
> >>> - bool preload = gfp & __GFP_WAIT;
> >>> + bool preload = !!(gfp & __GFP_WAIT);
> >>> int ret;
> >>>
> >>> /* If the id is already assigned, keep it. */
> >>> --
> >>
> >>
> >> I was wondering if the compiler still manages to optimise this in a
> >> manner that avoids actually calculating the boolean value...
> >>
> >> So I disassembled the compilation I just did of the old code (gcc 4.7.3).
> >> The object code looks strange.
>
> I'm not sure where you see this.
> Just tried with gcc 4.7.2 on x64 and assembler code is exactly the
> same before/after this change.
I was slightly worried it might generate the boolean value - something
that you really don't want it to do.
I only looked at the output for the old version.
The compiler seemed to have converted:
if (preload)
x();
y;
if (preload)
z();
into:
if (preload) {
x(); y; z();
} else {
y;
}
and then found out that z() was empty, leaving two copies of y().
David
> >> I think that idr_preload_end() must be an empty inline function.
> >
> >
> > Cc'ing Tejun. ;-)
> >
> >
> >> The compiler has duplicated the code between the two 'if (preload)'
> >> clauses (to avoid the conditional test), and the failed to tail
> >> merge everything in the latter stages.
> >> I suspect that an empty '#define' would generate smaller code.
> >>
> >> David
��.n��������+%������w��{.n�����{������ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
