[PATCH] D72224: [LegalizeVectorOps] Improve handling of multi-result operations.

[Kevin P. Neal via Phabricator via llvm-commits]

kpn added a comment.

I'm happy to see the uses of ReplaceAllUsesOfValueWith() gone. Having nodes be only halfway spliced into the tree for a while seemed like it could cause bugs later.  I imagine the scalar legalizer might could benefit from similar changes at some point in the future.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:495
+        AddLegalizedOperand(Op, Tmp);
+        return Tmp;
+      }
----------------
Is this a new case? Or was this a situation that could occur before your changes but was too subtle to see?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:502
+             "Unexpected number of results");
+      return TranslateLegalizeResults(Op, Tmp.getNode());
     }
----------------
It seems like a code smell to have the rules duplicated in the Promote, Expand, and Custom cases. Is there a way to pull most of this block out into a function shared by the three cases? And yet the Custom case looks a little different from the others. 


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:680
   if (IsStrict)
-    return DAG.getMergeValues({Promoted, Chain}, dl);
-
-  return Promoted;
+    Results.push_back(Chain);
 }
----------------
Nice. It's much cleaner than MERGE_VALUES.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72224/new/

https://reviews.llvm.org/D72224