[PATCH] D59039: [DAGCombine] Allow GatherAllAliases to pass through inline asm calls and glued nodes.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:19466
+      if (!IsLoad)
+        IsSafe = IsSafe && (EIInt & (InlineAsm::Extra_MayLoad));
+      if (IsSafe && hasGluedInputChain(C.getNode())) {
----------------
niravd wrote:
> efriedma wrote:
> > In terms of the actual checks here, is this sufficient to catch any inline asm that accesses memory?  Do you need to check for Extra_HasSideEffects?  Do you need to check for "indirect" operands separately?  (I think it's possible to write an indirect register operand in IR, although I'm not sure clang ever generates them...)
> SelectionDAGBuilder updates the inlineasm's ExtraInfo based on the asm parameter constraints so checking extra info should be should be sufficient.
> 
> Also, a check for HasExtraSideEffects is probably needed. I've updated the patch as such.
> 
With the change to check HasExtraSideEffects, can you remove the Mips test changes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59039