[llvm-dev] RFC: cleanup in Transforms/Utils

[Alina Sbirlea via llvm-dev]

Sent out: https://reviews.llvm.org/D48202

On Wed, Jun 13, 2018 at 2:19 PM, Alina Sbirlea <alina.sbirlea at gmail.com>
wrote:

> Let me add one more specific question.
> One method returns (does nothing) if BB.hasAddressTaken, while the other
> replaces the block address with constant 1? I have zero background of the
> usecase for the latter, so comments welcome.
>
> I do agree with having a general version, they each have some
> functionality the other does not, so the merge would be a superset of the
> two updating all analyses when available.
>
>
> On Wed, Jun 13, 2018 at 2:09 PM, Davide Italiano <dccitaliano at gmail.com>
> wrote:
>
>>
>>
>> On Wed, Jun 13, 2018 at 2:04 PM, Alina Sbirlea <alina.sbirlea at gmail.com>
>> wrote:
>>
>>> Hi,
>>>
>>> I'm looking to see what's the best way to solve the fact that these two
>>> utils mostly do the same thing:
>>> 1. lib/Transforms/Utils/Local.cpp : MergeBasicBlockIntoOnlyPred
>>> 2. lib/Transforms/Utils/BasicBlockUtils.cpp : MergeBlockIntoPredecessor
>>> (+cc some of the folks who touched at least one of these either
>>> originally or recently)
>>>
>>> Brief overview:
>>> 1. MergeBasicBlockIntoOnlyPred 2. MergeBlockIntoPredecessor
>>> Update DT Update DT
>>> Update either DT or DDT Updates LI and MemoryDependenceResults
>>> Move all instructions from Pred to BB, delete Pred Move all instruction
>>> from BB to Pred, delete BB
>>> assert BB has single predecessor return if BB doesn't have a single
>>> predecessor
>>>
>>> Can I get some background on why there are two methods with such similar
>>> functionality?
>>>
>>>
>> I can't comment on whether there are two methods, but I wanted to merge
>> them myself at some point (although other priorities showed up).
>> I think we should keep the more general version and it should preserve
>> the analyses.
>>
>> Thanks,
>>
>> --
>> Davide
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180614/b1970f5a/attachment.html>