[PATCH] D17625: Do not select EhPad BB in MachineBlockPlacement when there is regular BB to schedule

[Xinliang David Li via llvm-commits]

Two worklists work fine too -- the list traversal code can be properly
shared with different predicates.

David

On Wed, Mar 2, 2016 at 10:55 PM, Chandler Carruth <chandlerc at gmail.com>
wrote:

> On Thu, Mar 3, 2016 at 1:53 AM Xinliang David Li via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> On Wed, Mar 2, 2016 at 10:50 PM, Chandler Carruth <chandlerc at gmail.com>
>> wrote:
>>
>>> chandlerc added inline comments.
>>>
>>> ================
>>> Comment at: lib/CodeGen/MachineBlockPlacement.cpp:545
>>> @@ -543,1 +544,3 @@
>>>
>>> +    // We prefer non ehpad over ehpads.
>>> +    bool IsCandidateEHPad = MBB->isEHPad();
>>> ----------------
>>> davidxl wrote:
>>> > The new code really lowers the readability. Can you describe the
>>> algorithm here? Is it
>>> > 1) if the best non eh block exists, return it; otherwise
>>> > 2) return the least likely eh pad?
>>> >
>>> > if that is the case, why not simply skip eh blocks in the original
>>> worklist loop. If the best bb is found return it, otherwise have another
>>> helper loop to find the least likely eh pad?
>>> I agree that this makes it essentially unreadable. =/
>>>
>>> I'd suggest instead to maintain two worklists, one of non-EH-pads, and
>>> another of EH-pads. Then you can write the direct loop over each, and you
>>> can even put the algorithm David suggested in a comment on the second loop.
>>> =]
>>>
>>> Note that looping over the worklist twice could be quite expensive,
>>> which is why I'd suggest two lists.
>>>
>>
>> I assume that in majority of the cases, the first loop can find a
>> candidate, thus the second loop should be rarely executed.
>>
>
> Yes, but this makes it more quadratic than it already is.
>
> Plus, the EH blocks being *in* the first worklist loop makes that one
> slower skipping over the blocks.
>
> I really think the right design is to have separate more specialized data
> structures.
>
>
>>
>> David
>>
>>
>>>
>>>
>>> http://reviews.llvm.org/D17625
>>>
>>>
>>>
>>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160302/d8735716/attachment.html>