<html> <head> <meta http-equiv="Content-Type" content="text/html"/> </head> <body> <div style="color: black;"> On July 25, 2017 8:16:42 PM "Mun, Gwan-gyeong" <elongbug@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 <jason@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 <elongbug@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 <elongbug@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 </div> </body> </html>