This is an archive of the discontinued LLVM Phabricator instance.

Move the personality function from LandingPadInst to Function
ClosedPublic

Authored by majnemer on Jun 13 2015, 11:19 PM.

Details

Summary

The personality routine currently lives in the LandingPadInst.

This isn't desirable because:

  • All LandingPadInsts in the same function must have the same personality routine. This means that each LandingPadInst beyond the first has an operand which produces no additional information.
  • There is ongoing work to introduce EH IR constructs other than LandingPadInst. Moving the personality routine off of any one particular Instruction and onto the parent function seems a lot better than have N different places a personality function can sneak onto an exceptional function.

Diff Detail

Repository
rL LLVM

Event Timeline

majnemer updated this revision to Diff 27636.Jun 13 2015, 11:19 PM
majnemer retitled this revision from to Move the personality function from LandingPadInst to Function.
majnemer updated this object.
majnemer added a subscriber: Unknown Object (MLST).
qcolombet edited edge metadata.Jun 15 2015, 10:28 AM

Hi David,

Just to let you know that I’ve been added as a reviewer because of my Herald’s rule when a file has RegAlloc in it, which is the case for one of the test case you edited.
In particular, I do not feel like I am the one that should review this patch, therefore, you will need to find someone else to actually review this patch.

Cheers,
-Quentin

majnemer updated this revision to Diff 27708.Jun 15 2015, 1:13 PM
majnemer edited edge metadata.

Rebase to trunk.

rnk accepted this revision.Jun 15 2015, 3:48 PM
rnk added reviewers: rnk, rjmccall, asl, rafael.
rnk edited edge metadata.

I'm happy with this, but we should find another reviewer or two. I'd try Anton for EH stuff and John and Rafael for IR changes.

lib/Bitcode/Reader/BitcodeReader.cpp
4063 ↗(On Diff #27708)

What should we do with bitcode that has multiple landing pads with mismatched personalities? Obviously, it's invalid IR, but now we might not detect that anymore, right? Seems like if the parent function has a personality and it doesn't match the one we're adding, we should error.

This revision is now accepted and ready to land.Jun 15 2015, 3:48 PM
majnemer updated this revision to Diff 27773.Jun 16 2015, 10:51 AM
majnemer edited edge metadata.
  • Reserve exactly NumReservedValues
  • Address @rnk's review comments
rafael accepted this revision.Jun 16 2015, 1:31 PM
rafael edited edge metadata.

Why do you need a personality on declarations?

With that explained or changed this is fine by me.

docs/LangRef.rst
651 ↗(On Diff #27773)

Why do you need it on declarations?

andrew.w.kaylor accepted this revision.Jun 16 2015, 4:10 PM
andrew.w.kaylor added a reviewer: andrew.w.kaylor.
andrew.w.kaylor added a subscriber: andrew.w.kaylor.

Looks good to me.

docs/LangRef.rst
7301 ↗(On Diff #27773)

You should probably mention here that the personality function is inherited from the enclosing function.

majnemer updated this revision to Diff 27812.Jun 17 2015, 12:26 AM
majnemer edited edge metadata.
  • Reserve exactly NumReservedValues
  • Address @rnk's review comments
  • Address latest review feedback.
This revision was automatically updated to reflect the committed changes.