[PATCH] D69120: [TargetLowering][DAGCombine][MSP430] Shift Amount Threshold in DAGCombine (2/2)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/test/CodeGen/MSP430/shift-amount-threshold.ll:50
 
 define i16 @testExtendSignBit(i16 %a) {
 ; CHECK-LABEL: testExtendSignBit:
----------------
spatel wrote:
> Do we have test coverage for each code diff? 
> It would be good to add a code comment for each test that describes the transform that we are trying to avoid.
> For example, this one could copy the formula from the corresponding source code comment:
>   // sext i1 (setgt iN X, -1) --> sra (not X), (N - 1)
> 
Also, the IR does not correspond exactly to that comment. It's fine to include tests with 'select', but if there is an intermediate transform that turns it into 'sext' or 'zext', then we should have a test that begins with that pattern in IR too if possible. We want to make sure that we do not accidentally bypass these changes with future patches.


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

https://reviews.llvm.org/D69120