Skip to content

nir: fix and clean up metadata preservation (part 1)

Marcin Ślusarz requested to merge mslusarz/mesa:metadata-preserve into main

There's a lot of inconsistency in metadata preservation, sometimes leading to bugs, sometimes to wasted CPU time:

  • some passes don't call nir_metadata_preserve(..., nir_metadata_all) when they don't make progress (it works because metadata flags are validated only when pass makes progress, so it's not a fatal bug, but we can't verify that the pass cares about metadata)
  • some passes invalidate metadata when they don't make progress
  • some passes invalidate metadata of all functions after the first one
  • some passes ignore metadata completely and get away with it because a) per-pass check is disabled (it doesn't use NIR_PASS_V or NIR_PASS) and b) successive passes fix it by accident
  • there's a lot of almost identical code, but sometimes with subtle bugs added

To fix this and make metadata invalidation more robust I'm planning to:

  • fix the generic helper (nir_shader_instructions_pass) to not invalidate metadata of functions after the first one (done, merged via !12324 (merged))
  • convert as many users of nir_metadata_preserve as possible to nir_shader_instructions_pass (partially done)
  • make sure the remaining passes call nir_metadata_preserve(..., nir_metadata_all) when they don't make progress (partially done)
  • enable metadata validation in NIR_PASS when pass doesn't make progress (not done)
  • convert users of NIR_PASS_V to NIR_PASS or introduce metadata validation to NIR_PASS_V (not done , not sure yet which one is better)
  • in debug mode verify pass' claim of metadata correctness by checking that recalculated metadata is the same (not done, but @jekstrand has some code for that)

At the time I'm making this MR, diffstat looks like this:

39 files changed, 950 insertions(+), 1534 deletions(-)

Note 1: each commit's description should mention whether there are any functional changes.

Note 2: I may not be able to even compile some non-x86 drivers, so I'd appreciate some help with them.

Note 3: I recommend turning off "Show whitespace changes" ("gear" icon in the upper right corner of the commit view) to make this MR easier to review.

Reviewed patches merged via: !12324 (merged) !12467 (closed) !13176 (merged) !13189 (merged)

Edited by Marcin Ślusarz

Merge request reports