sgraenitz marked an inline comment as done.
sgraenitz added inline comments.
================
Comment at: llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp:597
+ assert(CV.size() > 0 && "Uncolored block");
+ for (BasicBlock *EHPadBB : CV)
+ if (auto *EHPad = dyn_cast<FuncletPadInst>(EHPadBB->getFirstNonPHI()))
{
----------------
ahatanak wrote:
> This piece of code does something similar to `cloneCallInstForBB`, but it's
> slightly different from it. It iterates over the entries in `CV` whereas the
> code above just looks at the first entry.
>
> Is this a bug that is fixed in https://reviews.llvm.org/D137945?
Yes, D137945 unifies the behavior so that we will always iterate. The unique
color invariant only holds after WinEHPrepare. So the old assertion in
`cloneCallInstForBB()` was formally wrong, but I only ever observed cases of
non-unique coloring in incorrect IR, i.e. dangling funclet tokens before
D137939. Iteration is the safe solution, especially since we may not be able to
bail out on dangling funclet tokens in the verifier (see
https://reviews.llvm.org/D138123#4067201).
FYI: We discussed it this in a previous round of this review
https://reviews.llvm.org/D137944?id=475132#inline-1334823
================
Comment at: llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp:751
- const ColorVector &CV = BlockColors.find(&BB)->second;
- assert(CV.size() == 1 && "non-unique color for block!");
- Instruction *EHPad = CV.front()->getFirstNonPHI();
----------------
The old code here assumed all EH coloring to be unique.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137944/new/
https://reviews.llvm.org/D137944
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
