[PATCH] D54803: [x86] promote all multiply i8 by constant to i32

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:41066-41077
+  // i16 is legal, but undesirable since i16 instruction encodings are longer
+  // and some i16 instructions are slow.
+  if (VT != MVT::i16) {
+    // 8-bit multiply is probably not much cheaper than 32-bit multiply, and
+    // we have specializations to turn 32-bit multiply into LEA or other ops.
+    if (VT != MVT::i8 || Opc != ISD::MUL)
+      return true;
----------------
I'm trying to parse this and i'm failing.

The old code was:
* If the type is not `i16`, then it is desirable. (so i8/i32/i64)
* Else, see `switch`.

New code says:
* If the type is not `i16` (so i8/i32/i64)
  and either
  * the type is not `i8` (so i32/i64)
  or
  * [the type is `i8` and] the opcode is not `ISD::MUL`
  then it is desirable
* Else, see `switch`.

Doesn't this not only mark i8 MUL's as undesirable, but also mark all other i8 as desirable?

Can this code please at least be uncondenced?


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

https://reviews.llvm.org/D54803