Re: [PATCH v7 12/35] Hexagon (target/hexagon) instruction attributes

[Philippe Mathieu-Daudé]

Hi Taylor,

+Eric in case I'm wrong.

On 1/30/21 12:15 AM, Taylor Simpson wrote:
>>>> On 1/20/21 4:28 AM, Taylor Simpson wrote:
>>>>> Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
>>>>> ---
>>>>>  target/hexagon/attribs.h     | 30 ++++++++++++++
>>>>>  target/hexagon/attribs_def.h | 95
>>>> ++++++++++++++++++++++++++++++++++++++++++++
>>>>>  2 files changed, 125 insertions(+)
>>>>>  create mode 100644 target/hexagon/attribs.h
>>>>>  create mode 100644 target/hexagon/attribs_def.h
>>>>>
>>>>> diff --git a/target/hexagon/attribs.h b/target/hexagon/attribs.h
>>>>> new file mode 100644
>>>>> index 0000000..e88e5eb
>>>>> --- /dev/null
>>>>> +++ b/target/hexagon/attribs.h
>>>>> @@ -0,0 +1,30 @@
>>>>> +
>>>>> +enum {
>>>>> +#define DEF_ATTRIB(NAME, ...) A_##NAME,
>>>>> +#include "attribs_def.h"
>>>>
>>>> Per QEMU conventions, this file has to be named "attribs_def.h.inc".
>>>
>>> Didn't know that.  Which files should end in .inc?
>>
>> Oh you are right, it is not documented in CODING_STYLE.rst.
>>
>> You can see the rationale in commits:139c1837db7 and 0979ed017f0:
>>
>>   meson: rename included C source files to .c.inc
>>
>>   With Makefiles that have automatically generated dependencies, you
>>   generated includes are set as dependencies of the Makefile, so that they
>>   are built before everything else and they are available when first
>>   building the .c files.
>>
>>   Alternatively you can use a fine-grained dependency, e.g.
>>
>>           target/arm/translate.o: target/arm/decode-neon-shared.inc.c
>>
>>   With Meson you have only one choice and it is a third option, namely
>>   "build at the beginning of the corresponding target"; the way you
>>   express it is to list the includes in the sources of that target.
>>
>>   The problem is that Meson decides if something is a source vs. a
>>   generated include by looking at the extension: '.c', '.cc', '.m', '.C'
>>   are sources, while everything else is considered an include---including
>>   '.inc.c'.
>>
>>   Use '.c.inc' to avoid this, as it is consistent with our other convention
>>   of using '.rst.inc' for included reStructuredText files.  The editorconfig
>>   file is adjusted.
> 
> OK, I understand why it's better to have files end .[ch].inc than .inc.[ch].
> 
> However, I need some confirmation on which files need .inc instead of simply 
> ending in .h.  From what I can tell these are the guidelines
> - If a file is intended to be included in the middle of another file (as 
> opposed to the top), it should end in .inc.

This has to be justified. Usually such file use macro definitions which
are defined by the file including them.

If you can not justify, there is probably a way to have your file as its
own .c/.h unit.

> - If a .inc file is intended to be included in a .h file, it should end in 
> .h.inc.

Yes, no exception.

> - If a .inc file is intended to be included in a .c file, it should end in 
> .c.inc.

Not necessarily, you can have .h.inc included in .c.inc:

Function prototype declarations -> .h
If generated -> .h.inc

Function body definitions -> .c
These can NOT go in .h, so if generated -> .c.inc

Inlined function body definitions -> .h/.c/.h.inc

> - The above applies to both human-written and generated files.

Yes, although it is harder to justify human-written .inc.

Also:

Header exposing subsystem X API to other subsystems go in include/..X..h
(example include/hw/sd/sd.h)

Header sharing prototypes local to a particular subsystem go in X/..h
(example hw/sd/sdmmc-internal.h)

*.inc must not go in include/

Regards (and sorry for answering late),

Phil.