[PATCH] D48485: [InstCombine] allow shl+mul combos with shuffle (select) fold (PR37806)

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 27 05:55:36 PDT 2018


lebedev.ri added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1202-1203
 
-  BinaryOperator::BinaryOps Opc = B0->getOpcode();
-  Instruction *NewBO = ConstantsAreOp1 ? BinaryOperator::Create(Opc, X, NewC) :
-                                         BinaryOperator::Create(Opc, NewC, X);
+  Instruction *NewBO = ConstantsAreOp1 ? BinaryOperator::Create(Opc0, X, NewC) :
+                                         BinaryOperator::Create(Opc0, NewC, X);
 
----------------
spatel wrote:
> lebedev.ri wrote:
> > This is going to be confusing later on, given that we already have `Opc0` and `Opc1`.
> > 
> Is it just the abbreviated variable naming (damn that 80-col limit!)? If so, I could spell out 'Opcode' vs. 'Operand'. Or rearrange the logic some way?
> damn that 80-col limit!
It's actually great :)

> If so, I could spell out 'Opcode' vs. 'Operand'. Or rearrange the logic some way?
It is perfectly clear that `Opc0` means `op code 0`.
What i was talking about is that we have two of them, and yet use the same one in both cases.

So maybe doing
```
assert(Opc0 == Opc1);
unsigned Opc = Opc0;
```
and using it would be cleanest.


https://reviews.llvm.org/D48485





More information about the llvm-commits mailing list