[PATCH] D48662: [InstCombine] reverse canonicalization of binops to allow more shuffle folding

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1158
+    case Instruction::Shl: {
+      // shl X, C --> mul X, 1 << C
+      Constant *C;
----------------
```
// shl X, C --> mul X, (1 << C)
```


================
Comment at: lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1215-1216
       DropNSW = true;
+    BinopElts AltB0 = getAlternateBinop(B0, DL);
+    if (AltB0.Opcode && isa<Constant>(AltB0.Op1)) {
+      Opc0 = AltB0.Opcode;
----------------
1. Why are you checking `isa<Constant>()`? Right now that can be an assertion. Future-proofing?
2. How about just adding `BinopElts::operator bool() const` and doing
```
if(BinopElts AltB0 = getAlternateBinop(B0, DL))
```
Or `Optional<>` like @RKSimon suggests.
?


https://reviews.llvm.org/D48662