This is an archive of the discontinued LLVM Phabricator instance.

allow SSAUpdater to be used for Swift

Authored by bob.wilson on Apr 13 2016, 3:41 PM.



The SSAUpdater is already implemented as a template so that we can reuse it for both IR-level and Machine-level updates. This simple change adds a new PhiItT typedef to the traits so that it can also be used for Swift's SIL representation, where phis are separate from other instructions.

Diff Detail

Event Timeline

bob.wilson updated this revision to Diff 53632.Apr 13 2016, 3:41 PM
bob.wilson retitled this revision from to allow SSAUpdater to be used for Swift.
bob.wilson updated this object.
bob.wilson added reviewers: mehdi_amini, dexonsmith.
bob.wilson added a subscriber: llvm-commits.
mehdi_amini added inline comments.Apr 13 2016, 3:45 PM

The interface is somehow lying: PhiItT_end is not the end of the PHIs list.
(not that I have a great suggestion to solve that)

aschwaighofer added inline comments.

How about calling it PhiCandidateItT?

Could you add a unit test (with comments describing the use case) so this
isn't accidentally broken later?

Would it be possible to refactor the begin/end as ADL begin/end so they'd
work with range-based for? (Or I guess that doesn't work - iteration over
the basic block's phis is a separate range from iterating over the basic
block's instructions? (so there's no authoritative begin/end to provide))
Perhaps it could be refactored as a range type/functor/function instead?
(not sure if that's better)

mehdi_amini added inline comments.Apr 14 2016, 10:54 AM

At least it wouldn't be a lie :)

I wonder if it would be useful to have real Phi iterator in LLVM though?

I don't see a reasonable way to write a unit test without either creating a stub IR or making the typedef typename Traits::PhiItT public and then just testing the presence.

I think swift users will notice when this breaks and complain.

bob.wilson abandoned this revision.Jan 2 2018, 11:18 AM

Chandler came up with a much better solution for phi iterators in r303964.
I have proposed a similar change for machine-level IR in
Abandoning this one.