[llvm] [SLP] NFC. Replace TreeEntry::setOperandsInOrder with VLOperands. (PR #113880)

[Han-Kuan Chen via llvm-commits]

================
@@ -3267,8 +3267,18 @@ class BoUpSLP {
     void setOperand(ArrayRef<Value *> VL, const BoUpSLP &R,
                     bool RequireReorder = false) {
       VLOperands Ops(VL, R);
-      if (RequireReorder)
+      if (RequireReorder) {
         Ops.reorder();
+        if (auto *CI = dyn_cast<CallInst>(VL[0])) {
+          Intrinsic::ID ID = getVectorIntrinsicIDForCall(CI, R.TLI);
----------------
HanKuanChen wrote:

> Need to add a check that ID is actually an intrinsic and avoid this check for non-intrinsics

Why do we need to check the ID while the old code does not have to?

> Also, why this processing is enabled only if RequireReorder is true?

Because only the commutative intrinsics check it in the old code.

I think we should NOT add `isVectorIntrinsicWithScalarOpAtArg` check for `setOperand` (for commutative and non commutative intrinsics). The reason is it will cause some bugs. I will use `llvm/test/Transforms/SLPVectorizer/X86/extract_in_tree_user.ll` as an example here.
If we make non commutative intrinsics check `isVectorIntrinsicWithScalarOpAtArg`, then
1. `%add1` will not be added into `TreeEntry` (because the operand 1 for powi is scalar operand).
2. While SLP is trying to do `schedule`, SLP cannot call `DecrUnsched` because the operand of `llvm.powi` is not inside the `TreeEntry` (it is `isVectorIntrinsicWithScalarOpAtArg`).
```
          for (unsigned OpIdx = 0, NumOperands = TE->getNumOperands();
               OpIdx != NumOperands; ++OpIdx)
            if (auto *I = dyn_cast<Instruction>(TE->getOperand(OpIdx)[Lane]))
              DecrUnsched(I);
```
3. Eventually it triggers the assert
```
#if !defined(NDEBUG) || defined(EXPENSIVE_CHECKS)
  // Check that all schedulable entities got scheduled
  for (auto *I = BS->ScheduleStart; I != BS->ScheduleEnd; I = I->getNextNode()) {
    ScheduleData *SD = BS->getScheduleData(I);
    if (SD && SD->isSchedulingEntity() && SD->hasValidDependencies())
      assert(SD->IsScheduled && "must be scheduled at this point");
  }
#endif
```

Only `smul_fix`, `smul_fix_sat`, `umul_fix` and `umul_fix_sat` are non commutative intrinsic and also `isVectorIntrinsicWithScalarOpAtArg`. However, the third operand of intrinsics are `Constant`, which will not trigger the assert.

I think we should revert https://github.com/llvm/llvm-project/pull/113880/commits/664f2d333ee051c30693b95ff8c59cf262c35cce.

https://github.com/llvm/llvm-project/pull/113880