Hi Jason,
thanks for your kind explanation.
I has totally understood your intention.
( I don't mean to bother you, at first I just wanted to silence of
below coverity warning.
-------------------------------------------------------------------------------------------------------------------------------------------------------
Identical code for different branches (IDENTICAL_BRANCHES)
identical_branches: The same code is executed regardless of whether
devinfo->is_haswell is true, because the 'then' and 'else' branches
are identical. Should one of the branches be modified, or the entire
'if' statement replaced?
-------------------------------------------------------------------------------------------------------------------------------------------------------
}
Best regards,
Gwan-gyeong.
2017-07-27 4:30 GMT+09:00 Jason Ekstrand <ja...@jlekstrand.net>:
> On July 25, 2017 8:16:42 PM "Mun, Gwan-gyeong" <elong...@gmail.com> wrote:
>
>> Hi Jason,
>> You are right, as you commented, compilers can eliminate these
>> redundancies
>> easy.
>> However I think we don't need to generate redundant codes.
>
>
> The approach we generally take with generators like this is to not really
> care too much what the generated C code looks like at that much so long as
> it compiles down to what we want. We are far more concerned with keeping
> the generator itself as simple and easy to maintain as possible. If we make
> the C compiler do a little more work, that's considered an acceptable loss
> so long as the final compiled binary is good. The part of the code which is
> maintained by humans is the generator so the ease of maintenance of the
> generator is more important than the generated C code.
>
> This patch makes the generator more complicated just to improve the
> generated C even though it compiles to the same binary. As such, this patch
> makes the codebase *less* maintainable with no improvement to the generated
> binary. This is not something we want to do.
>
> --Jason
>
>
>> Best regards,
>> Gwan-gyeong
>>
>> 2017년 7월 26일 (수) 오전 12:34, Jason Ekstrand <ja...@jlekstrand.net>님이 작성:
>>
>>> Does the redundancy ends up mattering in any way? A decent optimizing
>>> compiler should easily be able to get rid of that for you.
>>>
>>> --Jason
>>>
>>>
>>> On July 25, 2017 2:51:31 AM Gwan-gyeong Mun <elong...@gmail.com> wrote:
>>>
>>> > Before, it generates functions like this,
>>> >
>>> > static inline uint32_t ATTRIBUTE_PURE
>>> > RENDER_SURFACE_STATE_RedClearColor_start(const struct gen_device_info
>>> *devinfo)
>>> > {
>>> > switch (devinfo->gen) {
>>> > case 10: return 384;
>>> > case 9: return 384;
>>> > case 8: return 255;
>>> > case 7:
>>> > if (devinfo->is_haswell) {
>>> > return 255;
>>> > } else {
>>> > return 255;
>>> > }
>>> > case 6: return 0;
>>> > case 5: return 0;
>>> > case 4:
>>> > if (devinfo->is_g4x) {
>>> > return 0;
>>> > } else {
>>> > return 0;
>>> > }
>>> > default:
>>> > unreachable("Invalid hardware generation");
>>> > }
>>> > }
>>> >
>>> > After, it generates fuctions without a redundant identical code for
>>> different
>>> > branches.
>>> >
>>> > static inline uint32_t ATTRIBUTE_PURE
>>> > RENDER_SURFACE_STATE_RedClearColor_start(const struct gen_device_info
>>> *devinfo)
>>> > {
>>> > switch (devinfo->gen) {
>>> > case 10: return 384;
>>> > case 9: return 384;
>>> > case 8: return 255;
>>> > case 7: return 255;
>>> > case 6: return 0;
>>> > case 5: return 0;
>>> > case 4: return 0;
>>> > default:
>>> > unreachable("Invalid hardware generation");
>>> > }
>>> > }
>>> >
>>> > Signed-off-by: Mun Gwan-gyeong <elong...@gmail.com>
>>> > ---
>>> > src/intel/genxml/gen_bits_header.py | 8 ++++++++
>>> > 1 file changed, 8 insertions(+)
>>> >
>>> > diff --git a/src/intel/genxml/gen_bits_header.py
>>> > b/src/intel/genxml/gen_bits_header.py
>>> > index 1b3504073b..8084facdb7 100644
>>> > --- a/src/intel/genxml/gen_bits_header.py
>>> > +++ b/src/intel/genxml/gen_bits_header.py
>>> > @@ -83,20 +83,28 @@ ${item.token_name}_${prop}(const struct
>>> gen_device_info
>>> > *devinfo)
>>> > case 10: return ${item.get_prop(prop, 10)};
>>> > case 9: return ${item.get_prop(prop, 9)};
>>> > case 8: return ${item.get_prop(prop, 8)};
>>> > +% if item.get_prop(prop, 7) == item.get_prop(prop, 7.5):
>>> > + case 7: return ${item.get_prop(prop, 7)};
>>> > +% else:
>>> > case 7:
>>> > if (devinfo->is_haswell) {
>>> > return ${item.get_prop(prop, 7.5)};
>>> > } else {
>>> > return ${item.get_prop(prop, 7)};
>>> > }
>>> > +% endif
>>> > case 6: return ${item.get_prop(prop, 6)};
>>> > case 5: return ${item.get_prop(prop, 5)};
>>> > +% if item.get_prop(prop, 4) == item.get_prop(prop, 4.5):
>>> > + case 4: return ${item.get_prop(prop, 4)};
>>> > +% else:
>>> > case 4:
>>> > if (devinfo->is_g4x) {
>>> > return ${item.get_prop(prop, 4.5)};
>>> > } else {
>>> > return ${item.get_prop(prop, 4)};
>>> > }
>>> > +% endif
>>> > default:
>>> > unreachable("Invalid hardware generation");
>>> > }
>>> > --
>>> > 2.13.3
>>> >
>>>
>>>
>>> --
>> Gwan-gyeong Mun
--
Gwan-gyeong Mun
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
