Consider merging values and targets fields on MIR switchInt?

[wesleywiser]

Writing some kinds of MIR passes that care about values and targets on switchInt can be a bit messy (for example, passes that remove dead branches). @oli-obk has the idea:

oli: I like it. The actual elimination is a bit convoluted, but I see no way around it. Maybe the values and targets fields of the SwitchInt variant could be merged into a single list, then one could use retain on the list

oli: the otherwise branch would then need a custom field, but that would be fine I think

oli: not sure how much other could would be affected by that (I think there's some code that just wants to know about all targets, but that could be helped by creating an accessor function returning an iterator)

[oli-obk]

The mentioned values and targets lists are https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/mir/terminator/enum.TerminatorKind.html#variant.SwitchInt.field.values

My sketch of an idea would keep the targets list, but make it a Vec<Branch> where struct Branch { value: u128, target: BasicBlock }. There'd be an additional otherwise: BasicBlock field in the SwitchInt variant. This will affect a lot of optimizations, but I expect that all changes will be very mechanical changes.

[eddyb]

@oli-obk isn't it the way it is today to optimize the space usage? (not saying that's a good reason, just that I thought it was intentional)

EDIT: also, there are some things that expect a Cow<[BasicBlock]> to exist for targets, so we'd have to allocate to get that.

[oli-obk]

Hmm... true, allocating a bunch of (u128, u32) after each other is more expensive than having two vectors. Also a bunch of the values fields get initialized with a slice from global memory.
Maybe just creating a wrapper type around these two fields and giving it an API that doesn't expose the internal representation would make more sense? We can also try making target a SmallVec since a lot of things have just two branches.

EDIT: also, there are some things that expect a Cow<[BasicBlock]> to exist for targets, so we'd have to allocate to get that.

these could probably also take an impl Iterator<item = BasicBlock> or similar

[eddyb]

@oli-obk The problem is that they're returned, so you'd need something like a many-cased enum of different iterators. It could work, it was just cleaner to use Cow<[_]> I think.

[oli-obk]

Maybe just creating a wrapper type around these two fields and giving it an API that doesn't expose the internal representation would make more sense? We can also try making target a SmallVec since a lot of things have just two branches.

Ok, so I think this is the only actionable item from this issue then.