[PATCH] D46749: [SelectionDAG]Reduce masked data movement chains and memory access widths

[Sam Parker via Phabricator via llvm-commits]

samparker added a comment.

Hi Diogo,

Thanks for adding the tests again, but they're still not exercising your change properly - I don't see how your testing the SHL, ROTL or ROTR paths. Also by not checking the input and output registers, using regex or otherwise, you're not testing that the data flow is correct.
This patch should also be split again: into the shift folding and the load reduction, dag combining can be a pain so its best to break into down as much as possible.

cheers,
sam



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4156
+      // TODO: Add SHL_PARTS SRA_PARTS SRL_PARTS
+      break;
+    default:
----------------
remove


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:6455
+    if (CanFold) {
+      // fold (srl (shl X, c), c) -> (c) if the X upper bits of c are known to
+      // be 0
----------------
Is this needed for the rest of the patch to work? I would have expected that once the above transformation has been performed, that the transform you've added below would later be performed by the existing code in ReduceLoadWidth.


https://reviews.llvm.org/D46749