[PATCH] D128474: [BOLT] Support multiple parents for split jump table

[Amir Ayupov via Phabricator via llvm-commits]

Amir added inline comments.


================
Comment at: bolt/lib/Core/BinaryContext.cpp:631
 
-    if (!BF.isSimple())
+    if (!JT->Parents[0]->isSimple())
       continue;
----------------
nhuhuan wrote:
> Amir wrote:
> > I think checking if the first parent is non-simple is fine, but a future-proof solution would be to check if any of parents is non-simple. 
> I also thought the same. There are two reasons why I keep it this way:
> (a) Not sure about why we need this check.
>      Possibly the assumption is non-simple fragments are not be optimized, thus we
>      don't have to make best effort on analyzing it. So it boils down to the meaning of
>      non-simple. Maksim argues non-simple functions should have complete references,
>      just that we cannot construct a complete CFG.
> (b) The previous calls to analyzeJumpTable and getOrCreateJumpTable already mark
>       all parents as "HasIndirectTargetToSplitFragment = true". I mistakenly thought that
>       was the same with all parents being non-simple.
> The bottom line is that I will update following your feedbacks.
I agree with your reasoning. Let's stick to keeping the check for now as it's a safe option, and remove it later separately.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128474/new/

https://reviews.llvm.org/D128474