[PATCH] D32563: Add LiveRangeShrink pass to shrink live range within BB.

Dehao Chen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 5 11:26:37 PDT 2017


danielcdh added a comment.

In https://reviews.llvm.org/D32563#747393, @andreadb wrote:

> When you move an instruction, you should also move its associated debug values.
>
> See for example how this is done by MachineSink in method MachineSinking::SinkInstruction() (see file MachineSink.cpp).
>  For every instruction INSN that is a candidate for sinking, MachineSink collects all the DBG_VALUE instructions which are associated to INSN.
>  When INSN is moved to a different location, all the associated debug values (if any) are moved too.
>  I think you should do something similar.


Sorry, did not address this in the last patch. Done now. Note that I do not want to touch unnecessary files in this change. So I simply copied the code from MachineSink.cpp, and will refactor it to a utility function in MachineInstr.h/.cpp in a follow-up patch.

> About your new test case. I think a MIR test would be even better. That said, I don't have a strong opinion (I am okay with your new test).
>  In case, could you please regenerate the CHECK lines using `update_llc_test_checks.py? The test should be small, so I don't expect too many CHECK lines inserted by the script.

Done, but I'm not sure if that's the right thing to do. update_llc_test_checks makes the test too fragile: any subtle change of the codegen would break it. And it hides what the author really want to test and follow-up patch writers may simply run the script and regenerate the test (without understanding what it's testing, or they need to spend much time on what the original purpose is). BTW, what's MIR test? could you point me to one example? Thanks.



================
Comment at: lib/CodeGen/LiveRangeShrink.cpp:112
+      ++Next;
+      if (MI->isPHI() || MI->isCompare() || MI->isDebugValue())
+        continue;
----------------
gberry wrote:
> Why are you skipping compares?
In many architectures, cmp should stay together with jmp instruction for performance purpose. We do not want to break this in this pass.


https://reviews.llvm.org/D32563





More information about the llvm-commits mailing list