Commit c7c5da6d authored by Fangrui Song's avatar Fangrui Song
Browse files

Reland "[StackColoring] Remap PseudoSourceValue frame indices via... 

Reland "[StackColoring] Remap PseudoSourceValue frame indices via MachineFunction::getPSVManager()""

Reland 7a8b0b15, with a fix that checks
`!E.value().empty()` to avoid inserting a zero to SlotRemap.

Debugged by rnk@ in https://bugs.chromium.org/p/chromium/issues/detail?id=1045650#c33

Reviewed By: rnk

Differential Revision: https://reviews.llvm.org/D73510
parent 7c90666d

llvm/include/llvm/CodeGen/PseudoSourceValue.h

+1 −2
Original line number Diff line number Diff line
@@ -93,7 +93,7 @@ public: 
/// A specialized PseudoSourceValue for holding FixedStack values, which must

/// include a frame index.

class FixedStackPseudoSourceValue : public PseudoSourceValue {

  int FI;

  const int FI;



public:

  explicit FixedStackPseudoSourceValue(int FI, const TargetInstrInfo &TII)
@@ -112,7 +112,6 @@ public: 
  void printCustom(raw_ostream &OS) const override;



  int getFrameIndex() const { return FI; }

  void setFrameIndex(int FI) { this->FI = FI; }

};



class CallEntryPseudoSourceValue : public PseudoSourceValue {

llvm/lib/CodeGen/StackColoring.cpp

+14 −2
Original line number Diff line number Diff line
@@ -960,6 +960,8 @@ void StackColoring::remapInstructions(DenseMap<int, int> &SlotRemap) { 
  }



  // Remap all instructions to the new stack slots.

  std::vector<std::vector<MachineMemOperand *>> SSRefs(

      MFI->getObjectIndexEnd());

  for (MachineBasicBlock &BB : *MF)

    for (MachineInstr &I : BB) {

      // Skip lifetime markers. We'll remove them soon.
@@ -1025,13 +1027,14 @@ void StackColoring::remapInstructions(DenseMap<int, int> &SlotRemap) { 
      SmallVector<MachineMemOperand *, 2> NewMMOs;

      bool ReplaceMemOps = false;

      for (MachineMemOperand *MMO : I.memoperands()) {

        // Collect MachineMemOperands which reference

        // FixedStackPseudoSourceValues with old frame indices.

        if (const auto *FSV = dyn_cast_or_null<FixedStackPseudoSourceValue>(

                MMO->getPseudoValue())) {

          int FI = FSV->getFrameIndex();

          auto To = SlotRemap.find(FI);

          if (To != SlotRemap.end())

            const_cast<FixedStackPseudoSourceValue *>(FSV)->setFrameIndex(

                To->second);

            SSRefs[FI].push_back(MMO);

        }



        // If this memory location can be a slot remapped here,
@@ -1071,6 +1074,15 @@ void StackColoring::remapInstructions(DenseMap<int, int> &SlotRemap) { 
        I.setMemRefs(*MF, NewMMOs);

    }



  // Rewrite MachineMemOperands that reference old frame indices.

  for (auto E : enumerate(SSRefs))

    if (!E.value().empty()) {

      const PseudoSourceValue *NewSV =

          MF->getPSVManager().getFixedStack(SlotRemap.find(E.index())->second);

      for (MachineMemOperand *Ref : E.value())

        Ref->setValue(NewSV);

    }



  // Update the location of C++ catch objects for the MSVC personality routine.

  if (WinEHFuncInfo *EHInfo = MF->getWinEHFuncInfo())

    for (WinEHTryBlockMapEntry &TBME : EHInfo->TryBlockMap)

llvm/test/CodeGen/PowerPC/stack-coloring-vararg.mir

+13 −1
Original line number Diff line number Diff line 
# RUN: llc -o - %s -start-before=stack-coloring

# RUN: llc -run-pass=stack-coloring %s -o - | FileCheck %s



## Test %stack.1 is merged into %stack.0 and there is no MemoryMemOperand

## referencing %stack.1. This regression test is sensitive to the StackColoring

## algorithm. Please adjust or delete this test if the merging strategy

## changes.



# CHECK:    {{^}}stack:

# CHECK-NEXT: - { id: 0,

# CHECK-NOT:  - { id: 1,

# CHECK:      - { id: 2,

# CHECK-NOT: %stack.1



--- |

  ; ModuleID = '<stdin>'

  source_filename = "<stdin>"