- User Since
- Oct 15 2012, 2:12 PM (240 w, 4 d)
Looks ok to me too, Matthias?
One minor nit and LGTM.
Haven't checked the numbers yet, but perhaps assert?
Wed, May 24
FWIW I think the flag "inline-generic-switch-cost" is both misleading (it takes true and not a number) and not helpful (the name of the flag doesn't tell me anything). Changing that would be nice. I'll let others determine whether or not we should turn it on now or not, the patch itself should be fine though.
Tue, May 23
I have one inline comment here, but it can be addressed in a follow up patch.
Sadly I think constant checking plus the shift makes it hard to do with just __builtin_shufflevector in altivec.h.
I probably would have added this as a feature in ARMTargetInfo similar to CRC/soft-float/etc.
One inline question, looks ok to me otherwise.
This works for me. Thanks!
A few cosmetic changes in addition to the notation change that was mentioned (I also like that).
Couple of small nits and a request to make some of the change separately, but otherwise LGTM. For the split part please don't actually submit another patch, just go ahead and do it :)
Mon, May 22
Wed, May 17
I'm not even really sure that +thumb_mode works in all cases. Why not just have a single TargetMachine at that rate for the ARM back end?
Feel free to make any changes like this in the future. :)
This LGTM. I'd get one from Chandler as well though.
A few inline comments, a couple of refactorings and some cleanup, otherwise looks OK.
Tue, May 16
Mon, May 15
Wed, May 10
If you can come up with a .ll testcase for the register class change that would be good. I'd like to get this in so don't spend a huge amount of time trying to reduce something.
Couple of small comment requests, but that's it.
Tue, May 9
Seems useful for a disassembler change (feel free to make it separately as well if there's no visible change here).
Is there anything going on here? Should we do this?
Mon, May 8
Few more inline comments :)
Fri, May 5
Might want to remove the NFC then :)
Catching up on code review. This is one of the next things on my list.
Thu, May 4
In general I'm a fan of the change, however, it feels like the heuristic value is somewhat unintelligable on scan. I.e. what's the difference between 1 << 13 and 5 (or any other value)? :)
I can think of three ways to enable this:
A quick question:
Tue, May 2
Let's delete any function that just returns false or SDValue() and see what that does to some of the code.
Thu, Apr 27
LGTM and cleans up some interactions nicely, I've gone ahead and added Justin here in case he wants to comment.
Wed, Apr 26
FWIW this is fixing a wrong code case for Power.
Testcase? Seems reasonable otherwise, but Matthias might have something more here.
Apr 26 2017
This is fine with me.
Apr 25 2017
(I do know the asm printer and this looks ok to me :)
Apr 17 2017
Apr 14 2017
Seems reasonable to me. Feel free to iterate until happy :)
Apr 11 2017
Looks like Adrian has this review, removing myself.
Sounds like Dave is asking for changes so I'll put it down as Request Changes to get it out of my queue. :)
Probably best to have Quentin or Matthias do it.
Were we going to pick this up again? Still seems useful.
Apr 10 2017
Apr 6 2017
Apr 4 2017
Totally happy to pull it back if you'd like, was just going on what Quentin said to do.
Apr 3 2017
Couple of inline comments. I think this looks right but I've added Kit and Nemanja on here so one of them can give an ACK if Bill doesn't.
Mar 31 2017
Mar 30 2017
Ehsan has left the building so I'm going to take this over and get it committed.
SGTM. Anything you can do to help manage complexity helps :)
This has been committed for some time.
Mar 29 2017
echristo@athyra ~/s/l/t/clang> git svn dcommit
Committing to https://llvm.org/svn/llvm-project/cfe/trunk ...