When lowering invoke statement, all unwind destinations are directly added as successors of call site block, and the weight of those new edges are not assigned properly. Actually, default weight 16 are used for those edges. This patch calculates the proper edge weights for those edges when collecting all unwind destinations.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Thanks for the reviewer recommendation, Hans!
Reid, could you please take a look at this patch?
This seems like the right approach. My first thought was to add logic similar to findUnwindDestinations in BPI::getEdgeWeight, but after thinking more I like this is better.
lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | ||
---|---|---|
1203 ↗ | (On Diff #36168) | This is just code that runs on every iteration of the while loop. We don't need a lambda, just put it before the end of the loop. You'll have to create a NewEHPadBB local variable and assign it to EHPadBB, but that seems OK to me. |
test/CodeGen/X86/catchpad-weight.ll | ||
1 ↗ | (On Diff #36168) | I think the IR for this C++ code would make a better test case: struct A {}; struct B {}; struct C {}; struct HasDtor { ~HasDtor(); }; void may_throw(); int main() { HasDtor o; try { may_throw(); } catch (A) { } catch (B) { } catch (C) { } } Then the invoke for may_throw has five total successors: the normal edge, the three catchpads, and the cleanup. You can get the catchpad IR like this: $ clang -c --target=x86_64-windows-msvc t.cpp -S -emit-llvm -o - -fexceptions -O2 |
lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | ||
---|---|---|
1203 ↗ | (On Diff #36168) | I have removed the lambda by putting it to the end of the loop (the else case should escape from this part). |
test/CodeGen/X86/catchpad-weight.ll | ||
1 ↗ | (On Diff #36168) | Yes, this test case is more complete. I have replaced the old test case with this one. Thanks! |
Update the patch according to Reid's comment. Adjust some test cases after merging with the up-to-date trunk version.
lgtm
Correcting the weights seems to have fixed the strange, non-source order block placement. Thanks!