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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added reviewers: efriedma, hfinkel, bogner, arsenm, t.p.northover, sdardis, asb.
spatel added a comment.
Herald added a subscriber: wdng.

Let me cross-ref back to the llvm-dev thread for the motivating example and perf results so far:
http://lists.llvm.org/pipermail/llvm-dev/2018-January/120522.html
...and add some reviewers who likely have better intuition about the potential backend effects on non-x86 targets.
Reminder: this should have no effect on x86 because i8/i16 are already data-layout-legal there.



================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:156-157
+  // extra combine opportunities.
+  if (FromLegal && ToWidth < FromWidth && (ToWidth == 8 || ToWidth == 16))
+    return true;
+
----------------
See Alex Bradbury's comment in the llvm-dev thread about i32 too. Maybe we generalize this for ToLegal as:
  bool ToLegal = ToWidth == 1 || isPowerOf2_32(ToWidth) || DL.isLegalInteger(ToWidth);


================
Comment at: test/Transforms/InstCombine/should-change-type.ll:3
+; RUN: opt < %s -instcombine -S | FileCheck %s
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+
----------------
Should be able to reduce that string to just: "n32" ?


================
Comment at: test/Transforms/InstCombine/should-change-type.ll:5
+
+; Tests for removing zext/trunk from/to i8 and i16, even if it is not a
+; legal type.
----------------
trunk -> trunc


https://reviews.llvm.org/D42424