[PATCH] D26149: [DAGCombiner] Match load by bytes idiom and fold it into a single load

[Artur Pilipenko via Phabricator via llvm-commits]

apilipenko added a comment.

@filcab, the idea was mentioned above that we might want to match patterns other that bswap, for example rotate. Given this I'd prefer to keep bit-by-bit matching.

As for improving the matching do you mean supporting the listed instructions as a part of the pattern? I do plan to add bswap and different extension nodes support in a follow up patch. I'd like to keep the initial patch simple though.



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4591
+                                        VT, FirstLoad->getAddressSpace(),
+                                        FirstLoad->getAlignment(), &Fast);
+  if (!Allowed || !Fast)
----------------
filcab wrote:
> Should we check this before we do all the work with the bitoffsets?
> Might be hard as we probably need address space + alignment info. But we can at least bubble this up to above the little/big endian decision (unsure if that's very profitable, though).
I don't think that it will make a difference since LE/BE matching is not that expensive. On the other hand this placement seems a bit more logical: we ask about a possibility of load from a particular location when we know that the code in question is actually doing this load. 


https://reviews.llvm.org/D26149