search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.94k stars 11.53k forks source link

ShrinkWrap pass uses wrong csr #42344

Open linzj opened 5 years ago

linzj commented 5 years ago
Bugzilla Link 42999
Version trunk
OS Linux
CC @francisvm,@MatzeB,@qcolombet

Extended Description

Index: lib/CodeGen/ShrinkWrap.cpp

--- lib/CodeGen/ShrinkWrap.cpp (revision 359070) +++ lib/CodeGen/ShrinkWrap.cpp (working copy) @@ -280,8 +280,8 @@ // separately. An SP mentioned by a call instruction, we can ignore, // though, as it's harmless and we do not want to effectively disable tail // calls by forcing the restore point to post-dominate them.

RCI.getLastCalleeSavedAlias(PhysReg) may work on many cases, but one target may specify other CSRs in its frame lowering implementation. For example, ARMFrameLowering.cpp has the following code: if (STI.isTargetWindows() && WindowsRequiresStackProbe(MF, MFI.estimateStackSize(MF))) { SavedRegs.set(ARM::R4); SavedRegs.set(ARM::LR); }

It add R4 to CSR set. So I suggest ShrinkWrap should use the patch above, which uses getCurrentCSRs to determine CSRs.

llvmbot commented 5 years ago

I get no idea how to use it? Any help?

It's a review tool for whole llvm project. https://reviews.llvm.org. You can see document at http://llvm.org/docs/Phabricator.html. And so is http://llvm.org/docs/DeveloperPolicy.html.

linzj commented 5 years ago

I get no idea how to use it? Any help?

llvmbot commented 5 years ago

Index: lib/CodeGen/ShrinkWrap.cpp

--- lib/CodeGen/ShrinkWrap.cpp (revision 359070) +++ lib/CodeGen/ShrinkWrap.cpp (working copy) @@ -280,8 +280,8 @@ // separately. An SP mentioned by a call instruction, we can ignore, // though, as it's harmless and we do not want to effectively disable tail // calls by forcing the restore point to post-dominate them.

  • UseOrDefCSR = (!MI.isCall() && PhysReg == SP) ||
  • RCI.getLastCalleeSavedAlias(PhysReg);
  • UseOrDefCSR =
  • (!MI.isCall() && PhysReg == SP) || getCurrentCSRs(RS).count(PhysReg); } else if (MO.isRegMask()) { // Check if this regmask clobbers any of the CSRs. for (unsigned Reg : getCurrentCSRs(RS)) {

RCI.getLastCalleeSavedAlias(PhysReg) may work on many cases, but one target may specify other CSRs in its frame lowering implementation. For example, ARMFrameLowering.cpp has the following code: if (STI.isTargetWindows() && WindowsRequiresStackProbe(MF, MFI.estimateStackSize(MF))) { SavedRegs.set(ARM::R4); SavedRegs.set(ARM::LR); }

It add R4 to CSR set. So I suggest ShrinkWrap should use the patch above, which uses getCurrentCSRs to determine CSRs.

You can put this on phabricator to get more feedback or contribute to codebase.