[PATCH] D71182: [NFC][llvm][MIRVRegNamerUtils] Making some stylistic changes to MIRVRegNamerUtils.cpp

Mon Dec 9 15:30:33 PST 2019

plotfi added a comment.

In D71182#1776159 <https://reviews.llvm.org/D71182#1776159>, @bogner wrote:

> In D71182#1776145 <https://reviews.llvm.org/D71182#1776145>, @plotfi wrote:
>
> > In D71182#1775875 <https://reviews.llvm.org/D71182#1775875>, @compnerd wrote:
> >
> > > The `std::transform` better expresses the intent IMO.  I think that it is definitely more idiomatic C++, so I think that is an improvement.
> >
> >
> > I agree with this. Was going through the changes in this util in my downtime and just saw some opportunities to spruce some things up. I find it really odd that you'd have a HashOperand lambda and not want to apply that to your collection of uses from a llvm::transform.
>
>
> FWIW I'm not really convinced that `std::transform` is *ever* clearer or better any more since `C++11` gave us `auto` and range-for.
>
> In any case, the rest of this all seems like a nice clean up to me.

Thanks for this. I understand what you mean. In any case, I dropped the other weird transform and am now reusing the method in MRI for renaming the vregs. I really want to drop that weird isDef/kill modification code that was added long ago, because I am not really sure the semantic implications of it and things were kind of an experiment when those 3 lines of code were put in long ago. Will land once I do the recommended StringRef change. @aditya_nandakumar @compnerd @bogner thank you for the review guys!

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71182/new/

https://reviews.llvm.org/D71182