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_block_index = 0x1,
|
||||||
nir_metadata_dominance = 0x2,
|
nir_metadata_dominance = 0x2,
|
||||||
nir_metadata_live_ssa_defs = 0x4,
|
nir_metadata_live_ssa_defs = 0x4,
|
||||||
|
nir_metadata_not_properly_reset = 0x8,
|
||||||
} nir_metadata;
|
} nir_metadata;
|
||||||
|
|
||||||
typedef struct {
|
typedef struct {
|
||||||
@@ -1891,8 +1892,12 @@ void nir_print_instr(const nir_instr *instr, FILE *fp);
|
|||||||
|
|
||||||
#ifdef DEBUG
|
#ifdef DEBUG
|
||||||
void nir_validate_shader(nir_shader *shader);
|
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
|
#else
|
||||||
static inline void nir_validate_shader(nir_shader *shader) { (void) shader; }
|
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 */
|
#endif /* DEBUG */
|
||||||
|
|
||||||
void nir_calc_dominance_impl(nir_function_impl *impl);
|
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;
|
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
|
||||||
|
@@ -179,8 +179,12 @@ brw_nir_lower_outputs(nir_shader *nir, bool is_scalar)
|
|||||||
}))
|
}))
|
||||||
|
|
||||||
#define OPT(pass, ...) _OPT( \
|
#define OPT(pass, ...) _OPT( \
|
||||||
|
nir_metadata_set_validation_flag(nir); \
|
||||||
this_progress = pass(nir ,##__VA_ARGS__); \
|
this_progress = pass(nir ,##__VA_ARGS__); \
|
||||||
progress = progress || this_progress; \
|
if (this_progress) { \
|
||||||
|
progress = true; \
|
||||||
|
nir_metadata_check_validation_flag(nir); \
|
||||||
|
} \
|
||||||
)
|
)
|
||||||
|
|
||||||
#define OPT_V(pass, ...) _OPT( \
|
#define OPT_V(pass, ...) _OPT( \
|
||||||
|
Reference in New Issue
Block a user