It is inefficient and error-prone to recognize legal shuffles and then have isel repeat the process. For example, if the DAG combiner changes a shuffle in between legalization and isel, it may stop being legal and break isel. By legalizing to target-specific DAG nodes, we avoid that possibility and also make it much easier to match the shuffles during isel. On Feb 25, 2011, at 6:01 PM, David A. Greene wrote:> David Greene <dag at cray.com> writes: > >> In ToT, LowerVECTOR_SHUFFLE for x86 has this code: >> >> if (X86::isUNPCKLMask(SVOp)) >> getTargetShuffleNode(getUNPCKLOpcode(VT) dl, VT, V1, V2, DAG); >> >> why would this not be: >> >> if (X86::isUNPCKLMask(SVOp)) >> return SVOp; > > Ok, I discovered that Bruno did this in revisions 112934, 112942 and > 113020 but the logs don't really make clear why. I did figure out that > I needed new SDNode defs for VUNPCKLPSY and VUNPCKLPDY and corresponding > patterns. Once I added them everything started working. > > I found this all very confusing because it appears there are now two > ways to match certain shuffle instructions in .td files: one through the > traditional shuffle operators like unpckl and shufp and another through > these special X86* operators. > > This is reflected in X86InstrSSE.td: > > "Traditional": > > defm VUNPCKLPS: sse12_unpack_interleave<0x14, unpckl, v4f32, memopv4f32, > VR128, f128mem, "unpcklps\t{$src2, $src1, $dst|$dst, $src1, $src2}", > SSEPackedSingle>, VEX_4V; > "New-style": > > def : Pat<(v4f32 (X86Unpcklps VR128:$src1, (memopv4f32 addr:$src2))), > (VUNPCKLPSrm VR128:$src1, addr:$src2)>, Requires<[HasAVX]>; > > I think these are basically the same pattern. > > What's the purpose of these special operators and patterns? > > -Dave > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Bob Wilson <bob.wilson at apple.com> writes:> It is inefficient and error-prone to recognize legal shuffles and then > have isel repeat the process. For example, if the DAG combiner > changes a shuffle in between legalization and isel, it may stop being > legal and break isel. By legalizing to target-specific DAG nodes, we > avoid that possibility and also make it much easier to match the > shuffles during isel.Why is DAG combiner creating illegal nodes after legalization? It runs in various modes. If it creates such a node, it's a bug I think. Doing the matching this way means that we have to duplicate patterns and/or we lose opportunities for further pattern matching (multiple shuffles, etc.). Is it really worth the maintenance cost? The isel matcher has to look over the tree either way. What exactly is the inefficiency cost? In the experience I just had, it is quite error-prone to have multiple tblgen patterns to match these things. The way things were before, there was a clean separation between checking/enforcing node legality and doing the final code selection, with isel being automatic through tblgen. That was nice. The current setup mixes the two and seems to result in more code in the form of additional tblgen patterns. We also lose the ability to do shuffle peeps or any other such things unless we teach the code about every type of special target node. It really doesn't seem worth it to me. -Dave
> In the experience I just had, it is quite error-prone to have multiple > tblgen patterns to match these things. The way things were before, > there was a clean separation between checking/enforcing node legality > and doing the final code selection, with isel being automatic through > tblgen. That was nice. The current setup mixes the two and seems to > result in more code in the form of additional tblgen patterns. We also > lose the ability to do shuffle peeps or any other such things unless we > teach the code about every type of special target node. > > It really doesn't seem worth it to me.In the way it was done before, every shuffle that we tried to match had to be checked twice (masks used to be checked during legalization and during isel by the tblgen patterns), this is done only once now (during legalization). Although we still match the node itself through tblgen patterns, the tablegen patterns are a lot more clear now, and we were able to remove lots of confusing code. The code used to be *very* fragile, during legalization there was no explicit rule or comments of what and when stuff needed to be matched (and changing a single rule would generate a code that would fail to match tblgen patterns later), all the logic was getting really hard to understand. The long term plan is: generate all target specific nodes during legalization, and once all logic is clear, we can go for more fancy stuff like having per-processor family shuffle tables describing the more profitable shuffles for a specific mask, and so on. The work stopped because of this bug: http://llvm.org/bugs/show_bug.cgi?id=8156, but IMHO the implementation of x86 shuffle matching is a lot more clear now then they used to be in the past. -- Bruno Cardoso Lopes http://www.brunocardoso.cc