Re: [PATCH] Move gen_* stubs from defaults.h to genflags

[Mikhail Maltsev <maltsevm at gmail dot com>]

On 10.06.2015 10:05, Richard Sandiford wrote:
>> +/* Structure which holds data, required for generating stub gen_* function.  */
> 
> No comma after "data"
> 
>> +/* These instructions require default stub function.  Stubs are never called.
> 
> "require a default"
> 
[snip]
> Seems like this is more naturally a hash_table rather than a hash_map.
> I think there's also a preference to avoid static constructor-based
> initialisation.
Fixed.

> There again, this is a generator, so those kinds of concerns aren't
> particularly important.  If we do keep the above though, I think we
> should put the hasher in hash-map-table.h now.  Otherwise these FIXMEs
> are just going to accumulate, and each time makes it less likely that
> any consolidation will actually happen.
Well, after changing hash_map to hash_table, the hasher class is no
longer identical to other hash traits classes. As for fixing other
occurrences, I think I'd better leave it for another patch.

On 10.06.2015 17:55, Trevor Saunders wrote:
>> +  /* Number of arguments (i.e., instruction operands).  */
>> +  int opno;
> 
> unsigned?
Fixed.

>> +  /* Set to true when generator is output, so no stub is needed.  */
>> +  bool done;
>> +};
>> +
>> +/* These instructions require default stub function.  Stubs are never called.
> 
> are the ones that don't call gcc_unreachable () called?
> 
Well, bootstrap on x86_64 passes without such calls, but in general
case, I think they may get called (comment from genflags.c:main):

  /* We need to see all the possibilities.  Elided insns may have
     direct calls to their generators in C code.  */

For example, this would work if result of gen_* function is passed
directly to some emit_pattern_* function (they can handle NULL pointers).

>> +/* Print out a dummy for generator for instruction NAME with NUM arguments
>> +   which either does nothing, or aborts (depending on UNREACHABLE).  */
> 
> I believe you should drop the first "for" in this sentence.
Fixed.

-- 
Regards,
    Mikhail Maltsev

Attachment: gen_defaults2.patch
Description: Text document

Attachment: gen_defaults.clog
Description: Text document