[PATCH] D66459: Make ShrinkWrap more consistent.

zuojian lin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 21 00:59:04 PDT 2019


linzj marked an inline comment as done.
linzj added inline comments.


================
Comment at: llvm/lib/CodeGen/ShrinkWrap.cpp:293
+      UseOrDefCSR =
+          (!MI.isCall() && PhysReg == SP) || getCurrentCSRs(RS).count(PhysReg);
     } else if (MO.isRegMask()) {
----------------
chill wrote:
> linzj wrote:
> > chill wrote:
> > > Why is that needed? The set of //actual//  CSRs ought to be always a subset of the set of //possible// CSRs.
> > > 
> > I am actually making LLVM a code generator for V8 turbofan. V8 needs to save a specific set of registers at the entry of a function depending on the type of this function. For example, a JS function needs to save r0, r1,r7 along with fp, lr. And a wasm function needs to spill r3 at sp - 16.
> > 
> > At this context, I need to add additional CSRs at frame lowering's assignCalleeSavedSpillSlots. So getCurrentCSRs maybe a super set of the set of possible CSRs.
> Have you considered adding a custom calling convention in the backend, instead ? 
> https://llvm.org/docs/WritingAnLLVMBackend.html#calling-conventions
> 
> That may also remove the need for specially handing sibling calls in your case.
> 
I do add new calling convections:
def CSR_V8CC : CalleeSavedRegs<(add LR, R11)>;

def CSR_V8SBCC : CalleeSavedRegs<(add LR, R11, R10, R9, R8, R7, R6, R5,
                                     (sequence "D%u", 31, 0))>;

The problem is the CSRs added in assignCalleeSavedSpillSlots function may still get clobbered in the function. That is when a function returns, it will not restore any of these CSRs. And a call will not list them in the regmask, so they still considered be clobbered.

But they are CSRs in the ShrinkWrap context. For example, If a JS function use/def r1 in the  a basic block but ShrinkWrap move the save block to one of  its successor, that will be a trouble.

Here's some background information:
I am a developer from UC Browser, and LLVM for V8 turbofan has released to users for months without troubles. I am trying to make the LLVM patch back to the upstream.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66459





More information about the llvm-commits mailing list