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

[Artur Pilipenko via llvm-commits]

apilipenko added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4344
+/// value of the bit is either unknown, zero or comes from memory.
+struct BitProvider {
+  enum ProviderTy {
----------------
RKSimon wrote:
> I don't see any reason why these types are not inside DAGCombiner::MatchLoadCombine
I guess it's a matter of style. We can move BitProvider type and collectBitProviders helper into MatchLoadCombine but I doubt that it will improve readability. 

I put them into anonymous namespace so as not to pollute namespace, but if you think that they should live in MatchLoadCombine I don't mind.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4391
+const Optional<ValueBitProviders>
+collectBitProviders(SDValue Op, bool CheckNumberOfUses = false) {
+  Optional<ValueBitProviders> Result;
----------------
RKSimon wrote:
> Why not move this into MatchLoadCombine as a lambda?
Same as above.


https://reviews.llvm.org/D26149