[PATCH] D84332: [DAGCombiner] Fold sext_inreg of a masked load into a signed extended masked load

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11131
+  if (MaskedLoadSDNode *Ld = dyn_cast<MaskedLoadSDNode>(N0)) {
+    if (ExtVT == Ld->getMemoryVT() && !LegalOperations &&
+        Ld->getExtensionType() != ISD::LoadExtType::SEXTLOAD) {
----------------
samtebbs wrote:
> dmgreen wrote:
> > I think you need to check for hasOneUse. And maybe with more of the checks from the above `fold (sext_inreg (zextload x))` case too, like checking types match and maybe for legal masked loads.
> Thanks, I do need to add those. But for checking if masked loads are legal, is that necessary if there is already a masked load in the DAG?
OK. tryToFoldExtOfMaskedLoad and the load fold above both have a isLoadExtLegal check. That does seem to make some assumptions, but they seem to be valid so far.

It would be a strange architecture that included zext/anyext masked loads, but didn't include sext masked loads. But even if just for consistency it sounds like it would be worth included that isLoadExtLegal check too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84332