[PATCH] D66459: Make ShrinkWrap more consistent.

Tue Aug 20 02:45:16 PDT 2019

chill requested changes to this revision.
chill added a comment.
This revision now requires changes to proceed.

Could you, please, give some code examples of the issues that you  see?

Also, the patch needs regression tests, which fail without the patch applied (these can server as examples too).

================
Comment at: llvm/lib/CodeGen/ShrinkWrap.cpp:275
   }
+  if (MI.isTerminator())
+    return false;
----------------
I don't believe this is correct. A terminator instruction can well use and/or def a CSR, e.g. an indirect branch, a table jump, a hw loop instruction, ...

================
Comment at: llvm/lib/CodeGen/ShrinkWrap.cpp:293
+      UseOrDefCSR =
+          (!MI.isCall() && PhysReg == SP) || getCurrentCSRs(RS).count(PhysReg);
     } else if (MO.isRegMask()) {
----------------
Why is that needed? The set of //actual//  CSRs ought to be always a subset of the set of //possible// CSRs.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66459