Details
Diff Detail
Event Timeline
This is generally great, I think the helper function would be useful outside of this source file, too.
lib/Transforms/Scalar/LoopInterchange.cpp | ||
---|---|---|
82 | This adapter should really be available in BasicBlock.h or somewhere more accessible if it doesn't exist already. Could you move it there (or elsewhere appropriate) and add a doxygen comment? | |
1000 | I'm assuming that dbg intrinsic calls don't read memory, so this change may be a noop. Feel free to keep it for readability though. |
lib/Transforms/Scalar/LoopInterchange.cpp | ||
---|---|---|
82 | + 1, I think having iterator_range<iterator> instructions() and ... instructions_nodbg() in BasicBlock would be nice. |
lib/Transforms/Scalar/LoopInterchange.cpp | ||
---|---|---|
82 | Ok great, I will move it there. Do you think we should just have a member function instructionsNodbg() or a function taking an instruction iterator and returning a filtered iterator? It's just a question how we want to compose it with things like reverse |
lib/Transforms/Scalar/LoopInterchange.cpp | ||
---|---|---|
82 | I think we should have both. We'd need 'skipDebugIntrinsics' to implement 'instructionsNodbg', and 'instructionsNodbg' would make it easier to write correct code going forwards. Wdyt of placing 'skipDebugIntrinsics' in IR/DebugInfo.h? |
lib/Transforms/Scalar/LoopInterchange.cpp | ||
---|---|---|
82 | I'm a bit late to the party here, but have been keeping an eye on this discussion. I really like this idea. What about exposing this in IR/Intrinisics.h instead? For instance, what if there were other intrinsics that we would like to skip over as well (e.g., lifetime.start, lifetime.end). In that case I can imagine (in the future) an iterator that takes a list of types to skip-over, like a parameter pack or something, although I'm not sure if that would work with make_filter_range, just a thought. |
lib/Transforms/Scalar/LoopInterchange.cpp | ||
---|---|---|
82 | Having an easy way to skip all kinds of instructions sounds like a great idea. I will try to put initial patches together in the next few days. |
This adapter should really be available in BasicBlock.h or somewhere more accessible if it doesn't exist already. Could you move it there (or elsewhere appropriate) and add a doxygen comment?