[PATCH] D137844: [DAG] Allow folding zext/sext into masked loads with multiple uses

[Benjamin Maxwell via Phabricator via llvm-commits]

benmxwl-arm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12120
+          User->getOpcode() == ISD::CopyToReg)
+        return false;
+      // Replacing a non-free truncate with another non-free truncate should
----------------
peterwaller-arm wrote:
> Is it necessary to bail out for these cases, or would they still be handled by the isTruncFree logic? If you want to have the FIXME, it may be worth referencing ExtendUsesToFormExtLoad.
I'm not sure it's covered by the isTruncFree logic. The SETCC case is handled for the other loads by keeping a list of the SETCC nodes, then extending their other operands to match the new extended load. 

(This is only done for sign-extension, otherwise the transform load transform is not applied). 

When there's live in/out nodes the number of SETCCs is used to decide if the transform is worth it.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12110
+  auto AllUsesCanBeReplaced = [&](SDValue V) {
+    bool isTruncFree = TLI.isTruncateFree(VT, N0.getValueType());
+    for (SDNode::use_iterator UI = V->use_begin(), UE = V->use_end(); UI != UE;
----------------
peterwaller-arm wrote:
> `V` is `N0`, so do you need to close over `N0`?
I also close over `VT` and `TLI`, so I guess I could just use the closed `N0` too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137844