Re: [Qemu-devel] [PATCH v10 04/13] qapi: Drop pointless gotos in generated code

[Eric Blake]

On 02/16/2016 09:27 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> There's no point in emitting a goto if the very next thing
>> emitted will be the label.  A bit of cleverness in gen_visit_fields()
>> will let us choose when to omit a final error check (basically,
>> swap the order of emitted text within the loop, with the error
>> check omitted on the first pass, then checking whether to emit a
>> final check after the loop); and in turn omit unused labels.
>>
>> The change to generated code is a bit easier because we split out
>> the reindentation of the remaining gotos in the previous patch.
>> For example, in visit_type_ChardevReturn_fields():
>>
>> |     if (visit_optional(v, "pty", &obj->has_pty)) {
>> |         visit_type_str(v, "pty", &obj->pty, &err);
>> |     }
>> |-    if (err) {
>> |-        goto out;
>> |-    }
>> |-
>> |-out:
>> |     error_propagate(errp, err);
>>
>> Signed-off-by: Eric Blake <address@hidden>
>>

>> @@ -1679,8 +1682,9 @@ def gen_visit_fields(members, prefix='', 
>> need_cast=False, skiperr=False,
>>              ret += mcgen('''
>>      }
>>  ''')
>> +
>> +    if members and not skiplast:
>>          ret += gen_err_check(skiperr=skiperr, label=label)
>> -
>>      return ret
> 
> Is saving a goto the compiler can easily eliminate worth complicating
> the code?

I'm fine with dropping 3 and 4/13 if you think they don't add anything
(and they do indeed make it more complicated to reason about when it is
safe to drop a goto, and furthermore when to omit the label because all
gotos were dropped).  The generated code will occupy more source code
bytes, but as you say, the optimizing compiler should not be bloating
the code any differently based on whether the goto is present or absent.

Okay, just in typing that, you've convinced me - ease of maintenance
should triumph over concise generated code.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature