Page MenuHomePhabricator


Authored by Prazek on May 19 2018, 6:53 AM.



This patch introduce new intrinsic - that was described in the
RFC: Devirtualization v2

Diff Detail


Event Timeline

Prazek created this revision.May 19 2018, 6:53 AM
Prazek updated this revision to Diff 147661.May 19 2018, 7:08 AM
  • Add deleted tests
Prazek updated this revision to Diff 147663.May 19 2018, 7:21 AM
  • small fixes in tests
Prazek updated this revision to Diff 147665.May 19 2018, 8:04 AM


Prazek updated this revision to Diff 147683.May 19 2018, 3:11 PM

introduced mayBeDynamicClass and added more tests

xbolva00 added inline comments.May 19 2018, 3:22 PM
778 ↗(On Diff #147683)
Prazek updated this revision to Diff 147684.May 19 2018, 3:55 PM

Changed comments

Prazek updated this revision to Diff 147688.May 19 2018, 4:47 PM

After rebasing

The changes to Clang generally seem reasonable, but I think you should split them into a separate commit from the commit that adds the intrinsic itself.

3858 ↗(On Diff #147691)

Please add a comment explaining why this is necessary. (I'm actually not sure why it is, because surely the invariant groups we generate don't contain assumptions about memory from fields, right?)

1626 ↗(On Diff #147691)

Again, comment.

1767 ↗(On Diff #147691)

I think assigning to IntToPtr would be clearer here. You're changing the value returned, not introducing a reason to early-exit.

And a comment, please, on all of these cases.

Prazek updated this revision to Diff 148313.May 23 2018, 4:15 PM
Prazek marked 2 inline comments as done.

Slitted commit into defining and using intrinsic

3858 ↗(On Diff #147691)

Short answer: you can only make virtual calls on a dynamic pointer that carries and you can't do anything other because it could leak the information about this pointer (which when used with comparison could break devirtualization).

rjmccall added inline comments.May 23 2018, 9:08 PM
3858 ↗(On Diff #147691)

Alright, sure.

Prazek added a comment.Jun 2 2018, 1:45 PM

friendly ping

Prazek edited reviewers, added: davide; removed: rjmccall.Jun 13 2018, 6:43 AM
rsmith accepted this revision.Jun 28 2018, 2:37 PM
rsmith added inline comments.
12928 ↗(On Diff #148313)

does carries -> carries

This revision is now accepted and ready to land.Jun 28 2018, 2:37 PM
kuhar accepted this revision.Jul 1 2018, 3:47 PM
Closed by commit rL336073: Implement (authored by Prazek, committed by ). · Explain WhyJul 1 2018, 9:54 PM
This revision was automatically updated to reflect the committed changes.
Prazek marked an inline comment as done.