[PATCH] D34569: [DAG] Relax type restriction for store merge

Simon Pilgrim via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 4 08:37:12 PDT 2017


RKSimon added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:484
     /// Candidate stores have indirect dependency through their
-    /// operands. \return True if safe to merge
+    /// operands. \return True if safe to merge.
     bool checkMergeStoreCandidatesForDependencies(
----------------
A lot of these comment changes look like NFCs that can go in separately.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12476
+          while (Val.getOpcode() == ISD::BITCAST)
+            Val = Val.getOperand(0);
+          // Deal with constants of wrong size.
----------------
Please can you create a static helper that performs this? We have a lot of peek through bitcasts in this patch alone....


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12478
+          // Deal with constants of wrong size.
+          if (ElementSizeBytes * 8 != Val.getValueType().getSizeInBits()) {
+            EVT IntMemVT =
----------------
Val.getValueSizeInBits()


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12514
+          if (Val.getOpcode() == ISD::EXTRACT_VECTOR_ELT ||
+              Val.getOpcode() == ISD::EXTRACT_SUBVECTOR) {
+            SDValue Vec = Val.getOperand(0);
----------------
Merge these ifs() to reduce indentation?
```
if (MemVT != Val.getValueType() &&
    (Val.getOpcode() == ISD::EXTRACT_VECTOR_ELT ||
     Val.getOpcode() == ISD::EXTRACT_SUBVECTOR) {
```


https://reviews.llvm.org/D34569





More information about the llvm-commits mailing list