Based ontop of D104598, which is a NFCI-ish refactoring.
Here, a restriction, that only empty blocks can be merged, is lifted.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I think this is reasonable: later cfg simplifications will tail merge the preceding instructions and delete dbg.values appropriately. I didn't hit "accept" since the tests don't all pass yet.
llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-condbr-lower-tree.ll | ||
---|---|---|
37 | This transform has the potential to make it harder to recognize some tail calls during codegen, but it's clear from this test that codegen is already smart enough to handle this case. I don't think any change is needed. | |
llvm/test/CodeGen/ARM/smml.ll | ||
168 | I can't understand why these checks disappeared. Is this a bug in the check generation script? | |
llvm/test/Transforms/PruneEH/ipo-nounwind.ll | ||
33 | Seems like another update bug. |
Thank you for taking a look!
This is a much warmer reaction than i was expecting.
Autogenerated a few more tests. Still a few more to go.
Some of the test changes here look really ropy, but the performance benchmarks I tested was entirely flat, and codesize neither improved nor regressed by any significant amount.
There are some tests in here, notably the ifcvt ones, that no longer seem to be testing what they were originally intending to test.
llvm/test/CodeGen/AArch64/arm64-instruction-mix-remarks.ll | ||
---|---|---|
26–27 | I don't know this test, but what happened to this store? | |
llvm/test/CodeGen/ARM/smml.ll | ||
66–67 | Is this left over? | |
67 | These look like they can share CHECK-V6V7? |
I'm pretty sure Clang generates a single return block, so this change alone probably won't affect any C/C++ benchmarks. Most of the affected tests are manually written IR, where LLVM developers tend to use multiple ret terminators for simplicity.
I believe Clang also uses a single resume block, but I could be wrong.
Ah. That would explain the performance being flat. Codesize is different enough to suggest something is changing, but it's not enough of a difference to worry about.
llvm/test/CodeGen/AArch64/swifterror.ll | ||
---|---|---|
429 | I noticed you auto-generated the tests for this file and the GlobalISel version. It looks like those tests intentionally only checked the relevant parts related to swifterror, but now the checks are much more verbose (and much more prone to needing updating, because they check lots of 'uninteresting things). Given that only a few lines out of ~2000 needed updating, I am wondering if it would be better to keep the hand-written checks and just update the failing line? Going forward, I think the auto-generated ones will need updating much more frequently now. Granted it is easy to update them, but it still creates noise in diffs and in theory would require careful checking. |
llvm/test/CodeGen/AArch64/swifterror.ll | ||
---|---|---|
429 | (The original checks also had comments interleaved to explain what is being checked. It looks like they are now all before the auto-generated check lines) |
This transform has the potential to make it harder to recognize some tail calls during codegen, but it's clear from this test that codegen is already smart enough to handle this case. I don't think any change is needed.