[lld] [NFC][lld-macho] Generate test bodies for icf-safe-thunk tests (PR #111927)

Ellis Hoag via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 11 10:02:29 PDT 2024


ellishg wrote:

> Some of the IR was manually edited for simplicity and to remove unnecessary parts (as requested by the reviewers) - ex: removing necessary debug info, function attributes.

I think it depends on whether you think the "input test code" is the LLVM IR or the CPP code. If you think the LLVM IR is main code being tested, then we should make it as simple as possible to read and update. That's why I initially wanted some attributes simplified. However, it seems that we are treating the CPP code in the comments as the "true" input test code. Basically, we would rather call `clang` from `lld/test` and not check in any LLVM IR at all. Instead, we have to hide the CPP code in the comments and regenerate the IR manually. This PR improves that process by using existing infra that is more reproducible. I also noticed the script does nice things like setting the cwd to `/proc/self/cwd` and removing the clang commit hash to prevent leaking information about your machine.

> But it is useful only when the test and the outcome are straightforward, which doesn't seem to be the case here, e.g. the excessive attributes generated could be confusing.


I know the IR is more complicated, but I'd argue that we should only need to review the CPP code, not the IR.

> BTW, I remember the script used to generate a leading line saying something like "this is auto generated, don't modify, blah", they don't do it anymore? Like [this one](https://github.com/llvm/llvm-project/blob/72fb37922577997f3666203dbdb2601f0fc97748/llvm/test/CodeGen/AArch64/aarch64_fnmadd.ll#L1-L2)

For some reason the `update_test_body.py` script does not use the `common` package which will prefix the test with that line. I'm not sure why, but because it's useful I'll add a custom not at the top of these tests.

https://github.com/llvm/llvm-project/pull/111927


More information about the llvm-commits mailing list