[Mesa-dev] [PATCH] intel/compiler: Add a file-level description of brw_eu_validate.c

Matt Turner mattst88 at gmail.com
Thu Jan 24 21:40:34 UTC 2019


On Thu, Jan 24, 2019 at 12:16 PM Francisco Jerez <currojerez at riseup.net> wrote:
>
> Matt Turner <mattst88 at gmail.com> writes:
>
> > ---
> >  src/intel/compiler/brw_eu_validate.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/intel/compiler/brw_eu_validate.c b/src/intel/compiler/brw_eu_validate.c
> > index a25010b225c..7f1580a5bb3 100644
> > --- a/src/intel/compiler/brw_eu_validate.c
> > +++ b/src/intel/compiler/brw_eu_validate.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright © 2015 Intel Corporation
> > + * Copyright © 2015-2019 Intel Corporation
> >   *
> >   * Permission is hereby granted, free of charge, to any person obtaining a
> >   * copy of this software and associated documentation files (the "Software"),
> > @@ -24,6 +24,18 @@
> >  /** @file brw_eu_validate.c
> >   *
> >   * This file implements a pass that validates shader assembly.
> > + *
> > + * The restrictions implemented herein are intended to verify that instructions
> > + * in shader assembly do not violate restrictions documented in the graphics
> > + * programming reference manuals.
> > + *
> > + * The restrictions are difficult for humans to quickly verify due to their
> > + * complexity and abundance.
> > + *
> > + * It is critical that this code is thoroughly unit tested because false
> > + * results it will lead developers astray, which is worse than having no
>
> Redundant "it".
>
> > + * validator at all. Patches to this file without corresponding unit tests (in
> > + * test_eu_validate.cpp) will be rejected.
>
> Strictly by that rule this patch should be rejected ;).  Maybe say
> "functional changes" instead of "patches"?  Other than that:
>
> Reviewed-by: Francisco Jerez <currojerez at riseup.net>

Fair point :)

Thanks!


More information about the mesa-dev mailing list