[PATCH] D123414: [AArch64] Break up larger shuffle-masks into legal sizes in getShuffleCost

Tue Apr 19 04:03:55 PDT 2022

dmgreen added a comment.

In D123414#3448091 <https://reviews.llvm.org/D123414#3448091>, @RKSimon wrote:

> @dmgreen Might llvm::processShuffleMasks from D115653 <https://reviews.llvm.org/D115653> be of use here?

Hmm. I'm not sure. I remember thinking I should give it a try, I remember deciding it wasn't a good idea, but I don't remember why now.  I think it won't give us the information we really need. We would need to process the mask going in, and then process ">=2 source" back into "2 source" and ">2 source" masks, as it will currently build >2 source masks out of 2 source masks. Plus going into and out of lambdas can make the structure difficult to follow, as I had this code it seemed simpler. Perhaps I've misunderstood how it should be used.

================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:2654
+          NumSources = 1;
+        } else if (NumSources == 1 && Source != Source1) {
+          Source2 = Source;
----------------
samtebbs wrote:
> I might be misunderstanding this part of the code but I don't see how `Source != Source1` will ever be true, since `Source1` is assigned to `Source` when `NumSources == 0`, which is always the case in the first iteration. Same for the other else-if below.
The first unique Sources we find will get into the first `if (NumSources == 0)`. The next time we find a `Source` that isn't `== Source1` we will get into this if, and if we find more than 2 unique `Sources`, we just start to accumulate the total number of sources. If the Source is already one of `Source1` or `Source2` then we have already counted it and don't need to process. it in these checks again. Because `NumSources` increases it is kind of like a state machine.

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

https://reviews.llvm.org/D123414