Page MenuHomePhabricator

standardize {pred,succ,use,user}_{size,empty} like in MBB
ClosedPublic

Authored by artagnon on Jan 12 2015, 5:43 PM.

Details

Summary

MachineBasicBlock has {pred,succ}_{size,empty}, and it would be nice to
have a version that operated on BasicBlock. While at it, add
implementations for use_, user_.

Diff Detail

Event Timeline

artagnon updated this revision to Diff 18065.Jan 12 2015, 5:43 PM
artagnon retitled this revision from to standardize {pred,succ,use,user}_{size,empty} like in MBB.
artagnon updated this object.
artagnon edited the test plan for this revision. (Show Details)
artagnon added a subscriber: Unknown Object (MLST).
sanjoy edited edge metadata.Jan 12 2015, 6:04 PM

I'm not sure how much utility this adds -- what is a situation where you could profitably use these?

I'm not sure how much utility this adds -- what is a situation where you could profitably use these?

I designed this specifically for http://reviews.llvm.org/D6830 -- I imagine there must be many other instances?

I'm not sure how much utility this adds -- what is a situation where you could profitably use these?

I designed this specifically for http://reviews.llvm.org/D6830 -- I imagine there must be many other instances?

I don't understand -- how would you use these in D6830?

I imagine there must be many other instances?

The case for this patch would be much stronger if you find a few of those instances and refactor them to use these *_size functions.

ab added a subscriber: ab.Jan 12 2015, 6:31 PM
ab added inline comments.
include/llvm/IR/CFG.h
103

This is needlessly expensive when there are preds. How about checking for _end directly? (same for succ)

include/llvm/IR/Value.h
290

Nit: range-based for? (user_size as well)

artagnon added inline comments.Jan 12 2015, 6:58 PM
include/llvm/IR/CFG.h
103

Agreed; thanks.

include/llvm/IR/Value.h
290

I'm dropping .size() actually: you'll see why in the next iteration.

artagnon updated this revision to Diff 18066.Jan 12 2015, 6:58 PM
artagnon edited edge metadata.

Drop .size() functions, because they don't make any sense; it's
cheaper to do std::distance(.begin(), .end()). Put in a bunch of
usecases to support the .empty().

atrick accepted this revision.Jan 12 2015, 7:15 PM
atrick edited edge metadata.

This latest version LGTM.

This revision is now accepted and ready to land.Jan 12 2015, 7:15 PM
artagnon closed this revision.Jan 12 2015, 7:49 PM