[PATCH] D117406: [DAGCombiner] Adjust some checks in DAGCombiner::reduceLoadWidth

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12226-12227
+      assert(ExtType != ISD::SEXTLOAD && "Don't replace sextload by zextload.");
+      ExtType = ISD::ZEXTLOAD;
+      ExtVT = EVT::getIntegerVT(*DAG.getContext(), MemoryWidth - ShAmt);
       return SDValue();
----------------
spatel wrote:
> Why set these variables if we are exiting the function? Can we specify in the comment which function does the narrowing of the load for this pattern?
Oh, that return statement looks like a mistake. I started out by simply returning here, but then I realized that it should be OK to use ZEXTLOAD here, as long as that assert never triggers about replacing sextload by zextload. The idea was to remove the return statement again, not including it in the patch.

The example with `(i64 (truncate (i96 (srl (load x), 64))))` would however be optimzed also without this special case. But then it would start off by first rewriting srl+load into a larger zextload, which later is combined with the truncate to form a smaller zextload. By removing the return here we would trigger the full fold already based on the truncate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117406