i965/nir: Validate that NIR passes call nir_metadata_preserve().

Failing to call nir_metadata_preserve() can have nasty consequences:
some pass breaks dominance information, but leaves it marked as valid,
causing some subsequent pass to go haywire and probably crash.

This pass adds a simple validation mechanism to ensure passes handle
this properly.  We add a new bogus metadata flag that isn't used for
anything in particular, set it before each pass, and ensure it *isn't*
still set after the pass.  nir_metadata_preserve will reset the flag,
so correct passes will work, and bad passes will assert fail.

(I would have made these functions static inline, but nir.h is included
in C++, so we can't bit-or enums without lots of casting...)

Thanks to Dylan Baker for the idea.

Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Jason Ekstrand <jason.ekstrand@intel.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
This commit is contained in:
Kenneth Graunke
2015-11-03 00:31:22 -08:00
committed by Jason Ekstrand
parent 7bc0978999
commit 9ff71b649b
3 changed files with 48 additions and 3 deletions

View File

@@ -1312,6 +1312,7 @@ typedef enum {
nir_metadata_block_index = 0x1,
nir_metadata_dominance = 0x2,
nir_metadata_live_ssa_defs = 0x4,
nir_metadata_not_properly_reset = 0x8,
} nir_metadata;
typedef struct {
@@ -1891,8 +1892,12 @@ void nir_print_instr(const nir_instr *instr, FILE *fp);
#ifdef DEBUG
void nir_validate_shader(nir_shader *shader);
void nir_metadata_set_validation_flag(nir_shader *shader);
void nir_metadata_check_validation_flag(nir_shader *shader);
#else
static inline void nir_validate_shader(nir_shader *shader) { (void) shader; }
static inline void nir_metadata_set_validation_flag(nir_shader *shader) { (void) shader; }
static inline void nir_metadata_check_validation_flag(nir_shader *shader) { (void) shader; }
#endif /* DEBUG */
void nir_calc_dominance_impl(nir_function_impl *impl);

View File

@@ -52,3 +52,39 @@ nir_metadata_preserve(nir_function_impl *impl, nir_metadata preserved)
{
impl->valid_metadata &= preserved;
}
#ifdef DEBUG
/**
* Make sure passes properly invalidate metadata (part 1).
*
* Call this before running a pass to set a bogus metadata flag, which will
* only be preserved if the pass forgets to call nir_metadata_preserve().
*/
void
nir_metadata_set_validation_flag(nir_shader *shader)
{
nir_foreach_overload(shader, overload) {
if (overload->impl) {
overload->impl->valid_metadata |= nir_metadata_not_properly_reset;
}
}
}
/**
* Make sure passes properly invalidate metadata (part 2).
*
* Call this after a pass makes progress to verify that the bogus metadata set by
* the earlier function was properly thrown away. Note that passes may not call
* nir_metadata_preserve() if they don't actually make any changes at all.
*/
void
nir_metadata_check_validation_flag(nir_shader *shader)
{
nir_foreach_overload(shader, overload) {
if (overload->impl) {
assert(!(overload->impl->valid_metadata &
nir_metadata_not_properly_reset));
}
}
}
#endif

View File

@@ -178,9 +178,13 @@ brw_nir_lower_outputs(nir_shader *nir, bool is_scalar)
this_progress; \
}))
#define OPT(pass, ...) _OPT( \
this_progress = pass(nir ,##__VA_ARGS__); \
progress = progress || this_progress; \
#define OPT(pass, ...) _OPT( \
nir_metadata_set_validation_flag(nir); \
this_progress = pass(nir ,##__VA_ARGS__); \
if (this_progress) { \
progress = true; \
nir_metadata_check_validation_flag(nir); \
} \
)
#define OPT_V(pass, ...) _OPT( \