[llvm] [RemoveDIs] Update migration doc to explain how to handle textual IR updates (PR #91725)

Stephen Tozer via llvm-commits llvm-commits at lists.llvm.org
Fri May 10 04:05:01 PDT 2024


https://github.com/SLTozer created https://github.com/llvm/llvm-project/pull/91725

Related review: https://github.com/llvm/llvm-project/pull/91724

This patch updates the RemoveDIs migration document to include details on the textual IR changes, including steps to update any downstream lit tests accordingly. These steps are the same as those used to update the lit tests in the LLVM/Clang lit tests, as detailed in the review linked above.

>From 9b295470eebb200a92765751a315a0a2e7946105 Mon Sep 17 00:00:00 2001
From: Stephen Tozer <stephen.tozer at sony.com>
Date: Fri, 10 May 2024 11:19:42 +0100
Subject: [PATCH] Update migration doc to explain how to handle textual IR
 updates

---
 llvm/docs/RemoveDIsDebugInfo.md | 105 +++++++++++++++++++++++++++++++-
 1 file changed, 104 insertions(+), 1 deletion(-)

diff --git a/llvm/docs/RemoveDIsDebugInfo.md b/llvm/docs/RemoveDIsDebugInfo.md
index 9e50a2a604aa6..48a58aa7a5811 100644
--- a/llvm/docs/RemoveDIsDebugInfo.md
+++ b/llvm/docs/RemoveDIsDebugInfo.md
@@ -24,12 +24,115 @@ The debug records are not instructions, do not appear in the instruction list, a
 
 # Great, what do I need to do!
 
-Approximately nothing -- we've already instrumented all of LLVM to handle these new records ("`DbgRecords`") and behave identically to past LLVM behaviour. We plan on turning this on by default some time soon, with IR converted to the intrinsic form of debug info at terminals (textual IR, bitcode) for a short while, before then changing the textual IR and bitcode formats.
+Very little -- we've already instrumented all of LLVM to handle these new records ("`DbgRecords`") and behave identically to past LLVM behaviour. This is currently being turned on by default, so that `DbgRecords` will be used by default in memory, IR, and bitcode.
+
+## API Changes
 
 There are two significant changes to be aware of. Firstly, we're adding a single bit of debug relevant data to the `BasicBlock::iterator` class (it's so that we can determine whether ranges intend on including debug info at the beginning of a block or not). That means when writing passes that insert LLVM IR instructions, you need to identify positions with `BasicBlock::iterator` rather than just a bare `Instruction *`. Most of the time this means that after identifying where you intend on inserting something, you must also call `getIterator` on the instruction position -- however when inserting at the start of a block you _must_ use `getFirstInsertionPt`, `getFirstNonPHIIt` or `begin` and use that iterator to insert, rather than just fetching a pointer to the first instruction.
 
 The second matter is that if you transfer sequences of instructions from one place to another manually, i.e. repeatedly using `moveBefore` where you might have used `splice`, then you should instead use the method `moveBeforePreserving`. `moveBeforePreserving` will transfer debug info records with the instruction they're attached to. This is something that happens automatically today -- if you use `moveBefore` on every element of an instruction sequence, then debug intrinsics will be moved in the normal course of your code, but we lose this behaviour with non-instruction debug info.
 
+## Textual IR Changes
+
+As we change from using debug intrinsics to debug records, any tools that depend on parsing IR produced by LLVM will need to handle the new format. For the most part, the difference between the printed form of a intrinsic and a record is trivial:
+
+1. An extra 2 spaces of indentation are added.
+2. The call `(tail|notail|musttail)? call void @llvm.dbg._` is replaced with `#dbg_`.
+3. The leading `metadata ` is removed from each argument to the intrinsic.
+4. The DILocation changes from being a debug attachment, `!dbg !<Num>`, to being the final intrinsic argument without a leading `!dbg`.
+
+Altogether, this gives you:
+
+```
+; Intrinsic:
+  call void @llvm.dbg.value(metadata i32 %add, metadata !10, metadata !DIExpression()), !dbg !20
+; Record:
+    #dbg_value(i32 %add, !10, !DIExpression(), !20)
+```
+
+### Test updates
+
+Any tests downstream of the main LLVM repo that test the IR output of LLVM may break as a result of the change to using records. Updating an individual test to expect records instead of intrinsics should be trivial, given the update rules above. Updating many tests may be burdensome however; to update the lit tests in the main repository, the following steps were used:
+
+1. Collect the list of failing lit tests into a single file, `failing-tests.txt`, separated by (and ending with) newlines.
+2. Use the following line to split the failing tests into tests that use update_test_checks and tests that don't:
+    ```
+    $ while IFS= read -r f; do grep -q "Assertions have been autogenerated by" "$f" && echo "$f" >> update-checks-tests.txt || echo "$f" >> manual-tests.txt; done < failing-tests.txt
+    ```
+3. For the tests that use update_test_checks, run the appropriate update_test_checks script - for the main LLVM repo, this was achieved with:
+    ```
+    $ xargs ./llvm/utils/update_test_checks.py --opt-binary ./build/bin/opt < update-checks-tests.txt
+    $ xargs ./llvm/utils/update_cc_test_checks.py --llvm-bin ./build/bin/ < update-checks-tests.txt
+    ```
+4. The remaining tests can be manually updated, although if there is a large number of tests then the following scripts may be useful; firstly, a script used to extract the check-line prefixes from a file:
+    ```
+    $ cat ./get-checks.sh
+    #!/bin/bash
+
+    # Always add CHECK, since it's more effort than it's worth to filter files where
+    # every RUN line uses other check prefixes.
+    # Then detect every instance of "check-prefix(es)=..." and add the
+    # comma-separated arguments as extra checks.
+    for filename in "$@"
+    do
+        echo "$filename,CHECK"
+        allchecks=$(grep -Eo 'check-prefix(es)?[ =][A-Z0-9_,-]+' $filename | sed -E 's/.+[= ]([A-Z0-9_,-]+).*/\1/g; s/,/\n/g')
+        for check in $allchecks; do
+            echo "$filename,$check"
+        done
+    done
+    ```
+    Then a second script to perform the work of actually updating the check-lines in each of the failing tests, with a series of simple substitution patterns:
+    ```
+    $ cat ./substitute-checks.sh
+    #!/bin/bash
+
+    file="$1"
+    check="$2"
+
+    # Any test that explicitly tests debug intrinsic output is not suitable to
+    # update by this script.
+    if grep -q "write-experimental-debuginfo=false" "$file"; then
+        exit 0
+    fi
+
+    sed -i -E -e "
+    /(#|;|\/\/).*$check[A-Z0-9_\-]*:/!b
+    /DIGlobalVariableExpression/b
+    /!llvm.dbg./bpostcall
+    s/((((((no|must)?tail )?call.*)?void )?@)?llvm.)?dbg\.([a-z]+)/#dbg_\7/
+    :postcall
+    /declare #dbg_/d
+    s/metadata //g
+    s/metadata\{/{/g
+    s/DIExpression\(([^)]*)\)\)(,( !dbg)?)?/DIExpression(\1),/
+    /#dbg_/!b
+    s/((\))?(,) )?!dbg (![0-9]+)/\3\4\2/
+    s/((\))?(, ))?!dbg/\3/
+    " "$file"
+    ```
+    Both of these scripts combined can be used on the list in `manual-tests.txt` as follows:
+    ```
+    $ cat manual-tests.txt | xargs ./get-checks.sh | sort | uniq | awk -F ',' '{ system("./substitute-checks.sh " $1 " " $2) }'
+    ```
+    These scripts dealt successfully with the vast majority of checks in `clang/test` and `llvm/test`.
+5. Verify the resulting tests pass, and detect any failing tests:
+    ```
+    $ xargs ./build/bin/llvm-lit -q < failing-tests.txt
+    ********************
+    Failed Tests (5):
+    LLVM :: DebugInfo/Generic/dbg-value-lower-linenos.ll
+    LLVM :: Transforms/HotColdSplit/transfer-debug-info.ll
+    LLVM :: Transforms/ObjCARC/basic.ll
+    LLVM :: Transforms/ObjCARC/ensure-that-exception-unwind-path-is-visited.ll
+    LLVM :: Transforms/SafeStack/X86/debug-loc2.ll
+
+
+    Total Discovered Tests: 295
+    Failed: 5 (1.69%)
+    ```
+6. Some tests may have failed - the update scripts are simplistic and preserve no context across lines, and so there are cases that they will not handle; the remaining cases must be manually updated (or handled by further scripts).
+
 # C-API changes
 
 All the functions that have been added are temporary and will be deprecated in the future. The intention is that they'll help downstream projects adapt during the transition period.



More information about the llvm-commits mailing list