Page MenuHomePhabricator

[ADT] - Add llvm::make_mapped_range
AbandonedPublic

Authored by grimar on Apr 10 2018, 8:02 AM.

Details

Summary

This adds a helper for convenience to create mapped iterator ranges.

For use in D45166.

Diff Detail

Event Timeline

dblaikie accepted this revision.Apr 10 2018, 8:05 AM

Seems fine to me

This revision is now accepted and ready to land.Apr 10 2018, 8:05 AM
espindola accepted this revision.Apr 10 2018, 8:37 AM

LGTM too.

arichardson added inline comments.
include/llvm/ADT/STLExtras.h
208

Will using Optional here make lld slower with D45166 and a Release+Assertions build?
Can the compiler optimize away all the Optional assertions? Maybe using [F](ItTy It) { llvm_unreachable("end() derefeneced"); return F(It); } is easier to optimize?

grimar added inline comments.Apr 10 2018, 9:05 AM
include/llvm/ADT/STLExtras.h
208

I do not think LLD can be slower because of using Optional or even something slower than that here.
Speaking about D45166, the number of elements in loops where this is used is always small (we are iterating over linker script tree nodes there).

FWIW, filter_iterator also use Optional.

Also, I think David is right and it would not work.

arichardson accepted this revision.Apr 10 2018, 9:36 AM

Seems like at -O2 clang 5.0.0 can optimize away all the optional checks (https://godbolt.org/g/rbGYrN)

Hah, wow - didn't know you could include LLVM headers in godbolt code...
that's super handy :D

Sorry, I am abandoning this. I think D45166 patch (for which this was written) will not be landed soon.
If anyone thinks this can be OK to land this one stand-alone for something, I'll be happy to do that.

grimar abandoned this revision.May 15 2018, 7:42 AM