[PATCH] D149668: [ShrinkWrap] Use underlying object to rule out stack access.

[Jonathon Penix via Phabricator via llvm-commits]

jpenix-quic added inline comments.


================
Comment at: llvm/lib/CodeGen/ShrinkWrap.cpp:286
+  if (MI.mayLoadOrStore() && (MI.isCall() || MI.hasUnmodeledSideEffects() ||
+                              !all_of(MI.memoperands(), IsKnownNonStackPtr)))
     return true;
----------------
Hi @fhahn, does this work as expected when MI.memoperands() is empty? If I understand correctly, all_of will return true in that case which I don't think is what we want (my understanding is we need to be conservative if memoperands() is empty). That said, I struggled to find a test where this causes an issue here, so I'm not sure if this is actually an issue in practice or if any possible situations are covered by the other checks, etc.

Do you have thoughts on this? If it is an issue, I imagine we could just add an extra check for the empty case or something similar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149668