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:

committed by
Jason Ekstrand

parent
7bc0978999
commit
9ff71b649b
@@ -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);
|
||||
|
@@ -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
|
||||
|
@@ -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( \
|
||||
|
Reference in New Issue
Block a user