[PATCH] D42424: [InstCombine] Allow common type conversions to i8/i16

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: test/Transforms/InstCombine/select-bitext.ll:117
+; CHECK-NEXT:    [[SEXT:%.*]] = shl i32 [[TRUNC]], 16
+; CHECK-NEXT:    [[TMP1:%.*]] = ashr exact i32 [[SEXT]], 16
 ; CHECK-NEXT:    [[EXT:%.*]] = select i1 %cmp, i32 [[TMP1]], i32 42
----------------
spatel wrote:
> dmgreen wrote:
> > Although longer, this produced the same final assembly as the old function on any target I tried.
> We canonicalize trunc+sext to shifts, and we have a similar fold for:
>     // ashr (shl (zext X), C), C --> sext X
> ...so I think we can add a fold for this too (guarded by shouldChangeType).
> 
> https://rise4fun.com/Alive/NVU
> 
> I'll post that for review, so we can eliminate this sequence as a potential cause of regressions.
Actually, adding a fold for this won't work because we transform the other way (to shifts) in InstCombineCasts:

```
DEBUG(dbgs() << "ICE: EvaluateInDifferentType converting expression type"
          " to avoid sign extend: " << CI << '\n');
    // We need to emit a shl + ashr to do the sign extend.

```
So if we try to reduce to the form with sext, we'll infinite loop on a case like this:


```
target datalayout = "n8:16:32"

define i16 @trunc_ashr(i32 %x) {
  %t = trunc i32 %x to i16
  %s = shl i16 %t, 8
  %a = ashr i16 %s, 8
  ret i16 %a
}
```

It doesn't show up in the test here because this file doesn't specify a data-layout.



https://reviews.llvm.org/D42424