[PATCH] D50638: [AsmPrinter] Look inside bundles and rely on FrameDestroy in calculateDbgValueHistory

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: include/llvm/CodeGen/TargetFrameLowering.h:286
+  /// Check if the target is updating FrameSetup and FrameDestroy attributes on
+  /// machine instructions.
+  virtual bool isFrameSetupAndFrameDestroySupported() const {
----------------
vsk wrote:
> Do you know which backends don't mark up epilog instructions with FrameDestroy? Is it feasible to fix the backends? Seems like `collectChangingRegsInBody` is just a more precise way of doing the job of `collectChangingRegs`: it'd be great to just have one copy of the logic.
Yes, the goal would be to make the hook return true in the future.

All I know right know is that turning this on by default makes lots of lit-tests fail. So even if the backend is updating FrameDestroy properly, we need to examine failing lit-tests, check if the target is using FrameDestroy, and update CHECK-statements when the result is improved.

A suggested deployment could be:
1) Add this hook (this patch).
2) For each in-tree target, examine if target is using FrameDestroy, if so override the hook and check/update lit-test. If not, start using FrameDestroy and override on hook.
3) When all in-tree targets are using collectChangingRegsInBody, we can remove the old collectChangingRegs (and getFirstEpilogueInst) functions.

A possible addition to step 3 is to add an assert that isFrameSetupAndFrameDestroySupported returns true. That would give out-of-tree targets a chance to detect that they need to adapt their code somehow. And then after awhile we can remove the hook completely.


Repository:
  rL LLVM

https://reviews.llvm.org/D50638