[PATCH] D12166: Assign weights to edges to jump table when lowering switch statement.

Cong Hou via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 26 10:53:06 PDT 2015


Hi Hans

Could you please take a look at this patch and see if it is ok to be
checked in? Thank you!


Cong

thanks,
Cong

On Fri, Aug 21, 2015 at 4:47 PM, Hans Wennborg <hans at chromium.org> wrote:

> hans added a comment.
>
> This looks fine to me, but it would be really nice to get tests here.
>
>
> ================
> Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:1900
> @@ -1899,3 +1899,3 @@
>
> -  addSuccessorWithWeight(SwitchBB, B.Default);
> -  addSuccessorWithWeight(SwitchBB, MBB);
> +  uint32_t DefaultWeight = getEdgeWeight(SwitchBB, B.Default);
> +  addSuccessorWithWeight(SwitchBB, B.Default, DefaultWeight);
> ----------------
> Does it get the DefaultWeight right here? (Not a criticism, just wondering
> if this ever worked.)
>
> ================
> Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8019
> @@ +8018,3 @@
> +        for (auto Succ : JumpMBB->successors())
> +          JumpWeight += getEdgeWeight(JumpMBB, Succ);
> +        uint64_t FallthruWeight = getEdgeWeight(CurMBB, Fallthrough);
> ----------------
> I think the sum of all the branch weights on a branch (in this case a
> SwitchInst) will not overflow an uint32_t. Since we're adding up weights
> from the same switch it should be safe to use uint32_t for JumpWeight and
> skip the ScaleWeights below.
>
>
> http://reviews.llvm.org/D12166
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150826/894c360a/attachment.html>


More information about the llvm-commits mailing list