[PATCH] D70042: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (4)

[Joan LLuch via Phabricator via llvm-commits]

joanlluch marked 4 inline comments as done.
joanlluch added inline comments.


================
Comment at: llvm/lib/Target/MSP430/MSP430ISelLowering.cpp:364
+                                                       unsigned amount) const {
+  return !(amount == 8 || amount == 9 || amount<=2);
 }
----------------
joanlluch wrote:
> spatel wrote:
> > This changes the MSP430 output for shift by 8 or 9? If yes, there should be tests that show this difference.
> I commited baseline tests to show this (D70083). I did not wait for review in this particular case, as I understand it's simple enough. (It does not modify any code) (I hope this is ok). 
> I will submit an updated patch for this.
After all the related previous patches, showing improvements for the special case of 8 and 9 bit shifts on the MSP430 is really borderline. But they can be found in a couple of cases. The intention of this series of patches is to provide additional hooks for targets that may benefit from them. In the particular case of the MSP430, it's possibly debatable whether 8 and 9 bit shifts should be systematically factored-in, but in the general case, I think that it is good that the TLI hook semantics provide more flexibility to targets. Please let me know how it looks now. Thanks


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

https://reviews.llvm.org/D70042