[PATCH] D137318: [llvm-diff] Fix false-positive diffs on forward-referencing phi nodes

Jannik Silvanus via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 3 11:23:22 PDT 2022


jsilvanus added a comment.

Hi David, thanks for looking into this change!

I'm aware that there is very little active development for `llvm-diff`, and I chose you and @void based on the patch introducing phi node diffing.

While experimenting with `llvm-diff` (for context see below), I noticed this issue. 
Since it seems to affect many inputs (essentially anything with a loop counter), and the fix seemed to be not too complicated, I decided to work on this.

My main interest, although I am not yet sure whether `llvm-diff` is the right tool for the job, is related to `update_test_checks.py`. I'd like to reduce diff sizes of lit tests caused by changed automatic variable names (`%15` -> `%16`) of updated lit tests, with the goal of simplifying reviews.
Say you have an automatically generated, large lit test. A small change in the generated IR, say one added line, changes all following identifier names, and leads to a huge diff of text IR.
When updating the lit test with `update_test_checks.py`, these changes are propagated to the `CHECK` lines, since the generated FileCheck capture names (e.g. `TMP15`) are also incrementally numbered.
I'd like to have a mode where `update_test_checks.py` tries to preserve existing capture names to minimize the diff size of changed tests. The work flow would be like this:

1. Implement LLVM change
2. Update lit tests, preserving capture names
3. Normalize capture names in lit tests using an ordinary `update_test_checks.py` call

The result of 1 + 2 would be one small commit that is the main interest of review. Step 3 would be in a different, possibly large commit that only contains the automatically generated capture renaming and would not needed to be reviewed in the same detail. Note that 3. is not absolutely necessary, but it seems to be nicer to have tests with sequentially named identifiers without any remainders of past versions of the test.

A possible implementation of the above would be to run both the old and new `opt` binary on the test, use `llvm-diff` to establish a mapping of variables, and use this correspondence to establish a mapping of FileCheck capture names.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137318/new/

https://reviews.llvm.org/D137318



More information about the llvm-commits mailing list