[all-commits] [llvm/llvm-project] 1c4955: [BasicBlockUtils] Add utility to remove redundant ...

[Björn Pettersson via All-commits]

  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 1c49553c19a7044fbbf4528b732926f19f210e54
      https://github.com/llvm/llvm-project/commit/1c49553c19a7044fbbf4528b732926f19f210e54
  Author: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
  Date:   2019-12-16 (Mon, 16 Dec 2019)

  Changed paths:
    M llvm/include/llvm/InitializePasses.h
    M llvm/include/llvm/LinkAllPasses.h
    M llvm/include/llvm/Transforms/Scalar.h
    M llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
    M llvm/lib/Transforms/Scalar/DCE.cpp
    M llvm/lib/Transforms/Scalar/Scalar.cpp
    M llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
    A llvm/test/Transforms/DCE/dbg-value-removal.ll

  Log Message:
  -----------
  [BasicBlockUtils] Add utility to remove redundant dbg.value instrs

Summary:
Add a RemoveRedundantDbgInstrs to BasicBlockUtils with the
goal to remove redundant dbg intrinsics from a basic block.

This can be useful after various transforms, as it might
be simpler to do a filtering of dbg intrinsics after the
transform than during the transform.
One primary use case would be to replace a too aggressive
removal done by MergeBlockIntoPredecessor, seen at loop
rotate (not done in this patch).

The elimination algorithm currently focuses on dbg.value
intrinsics and is doing two iterations over the BB.

First we iterate backward starting at the last instruction
in the BB. Whenever a consecutive sequence of dbg.value
instructions are found we keep the last dbg.value for
each variable found (variable fragments are identified
using the  {DILocalVariable, FragmentInfo, inlinedAt}
triple as given by the DebugVariable helper class).

Next we iterate forward starting at the first instruction
in the BB. Whenever we find a dbg.value describing a
DebugVariable (identified by {DILocalVariable, inlinedAt})
we save the {DIValue, DIExpression} that describes that
variables value. But if the variable already was mapped
to the same {DIValue, DIExpression} pair we instead drop
the second dbg.value.

To ease the process of making lit tests for this utility a
new pass is introduced called RedundantDbgInstElimination.
It can be executed by opt using -redundant-dbg-inst-elim.

Reviewers: aprantl, jmorse, vsk

Subscribers: hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D71478


  Commit: 632deb6bd04022945468faef2dcaa8c9fdf1b0fd
      https://github.com/llvm/llvm-project/commit/632deb6bd04022945468faef2dcaa8c9fdf1b0fd
  Author: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
  Date:   2019-12-16 (Mon, 16 Dec 2019)

  Changed paths:
    A llvm/test/Transforms/LoopRotate/dbg-value-duplicates-2.ll

  Log Message:
  -----------
  [LoopRotate] Add test case to show dbg value problem

Summary:
In commit d60f34c20a2f31335c8d5626e (llvm-svn 317128,
PR35113) MergeBlockIntoPredecessor was changed into
discarding some dbg.value intrinsics referring to
PHI values, post-splice due to loop rotation.

That elimination of dbg.value intrinsics does not
consider which dbg.value to keep based on the context.
Such as always keeping the one that comes first textually,
or the need to keep several of them in case the variable
is changing it's value several times inside the basic block.

In the past that hasn't been such a big problem since
CodeGenPrepare::placeDbgValues has moved the dbg.value
to be next to the PHI node anyway. But after commit
00e238896cd8ad3a7d7 CodeGenPrepare isn't doing that
any longer, so we need to be more careful when avoiding
duplicate dbg.value intrinsics in MergeBlockIntoPredecessor.

This patch is just a pre commit of the test case.

Subscribers: llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D71479


  Commit: e5f07080b8ac73dd573e81a186925a122ab8a39d
      https://github.com/llvm/llvm-project/commit/e5f07080b8ac73dd573e81a186925a122ab8a39d
  Author: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
  Date:   2019-12-16 (Mon, 16 Dec 2019)

  Changed paths:
    M llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
    M llvm/test/DebugInfo/NVPTX/debug-loc-offset.ll
    M llvm/test/Transforms/LoopRotate/dbg-value-duplicates-2.ll
    M llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-inlined.ll

  Log Message:
  -----------
  [BasicBlockUtils] Fix dbg.value elimination problem in MergeBlockIntoPredecessor

Summary:
In commit d60f34c20a2f31335c8d5626e (llvm-svn 317128,
PR35113) MergeBlockIntoPredecessor was changed into
discarding some dbg.value intrinsics referring to
PHI values, post-splice due to loop rotation.

That elimination of dbg.value intrinsics did not
consider which dbg.value to keep depending on the
context (e.g. if the variable is changing its value
several times inside the basic block).

In the past that hasn't been such a big problem since
CodeGenPrepare::placeDbgValues has moved the dbg.value
to be next to the PHI node anyway. But after commit
00e238896cd8ad3a7d7 CodeGenPrepare isn't doing that
any longer, so we need to be more careful when avoiding
duplicate dbg.value intrinsics in MergeBlockIntoPredecessor.

This patch replaces the code that tried to avoid duplicate
dbg.values by using the RemoveRedundantDbgInstrs helper.

Reviewers: aprantl, jmorse, vsk

Reviewed By: aprantl, vsk

Subscribers: jholewinski, hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D71480


Compare: https://github.com/llvm/llvm-project/compare/4f17b1784e94...e5f07080b8ac