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

[Alexey Bataev via llvm-commits]

================
@@ -8591,46 +8572,13 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
 
       TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
                                    ReuseShuffleIndices);
-      // Sort operands of the instructions so that each side is more likely to
-      // have the same opcode.
-      if (isCommutative(VL0)) {
-        ValueList Left, Right;
-        reorderInputsAccordingToOpcode(VL, Left, Right, *this);
-        TE->setOperand(0, Left);
-        TE->setOperand(1, Right);
-        SmallVector<ValueList> Operands;
-        for (unsigned I : seq<unsigned>(2, CI->arg_size())) {
-          Operands.emplace_back();
-          if (isVectorIntrinsicWithScalarOpAtArg(ID, I))
-            continue;
----------------
alexey-bataev wrote:

> But for `IntrinsicInst` which is NOT commutative, all arguments will be stored into a TreeEntry (and SLP will not call `buildTree_rec` for those arguments).
> 
> If we have to "avoid calling setOperand for isVectorIntrinsicWithScalarOpAtArg args" for commutative `IntrinsicInst`, we have to do something like the following patch.
> 
> ```
> diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
> index d8c596eb875d..54210cc0b149 100644
> --- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
> +++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
> @@ -3267,8 +3267,18 @@ private:
>      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);
> +          for (unsigned I : seq<unsigned>(CI->arg_size())) {
> +            if (isVectorIntrinsicWithScalarOpAtArg(ID, I))
> +              continue;
> +            setOperand(I, Ops.getVL(I));
> +          }
> +          return;
> +        }
> +      }
>        for (unsigned I :
>             seq<unsigned>(cast<Instruction>(VL[0])->getNumOperands()))
>          setOperand(I, Ops.getVL(I));
> ```
> 
> But it does not look elegant.

We should maintain correctness where possible and should not produce "noise" entries in the graph. They may cause unexpecteв side effects and cause some extra compiler crashes

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