Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

Differential D35844 Diff 108098 include/llvm/CodeGen/MachineBasicBlock.h

Changeset View

Standalone View

View Options

include/llvm/CodeGen/MachineBasicBlock.h

Show All 17 Lines
#include "llvm/ADT/ilist.h" #include "llvm/ADT/ilist.h"
#include "llvm/ADT/ilist_node.h" #include "llvm/ADT/ilist_node.h"
#include "llvm/ADT/iterator_range.h" #include "llvm/ADT/iterator_range.h"
#include "llvm/ADT/simple_ilist.h" #include "llvm/ADT/simple_ilist.h"
#include "llvm/CodeGen/MachineInstr.h" #include "llvm/CodeGen/MachineInstr.h"
#include "llvm/CodeGen/MachineInstrBundleIterator.h" #include "llvm/CodeGen/MachineInstrBundleIterator.h"
#include "llvm/IR/DebugLoc.h" #include "llvm/IR/DebugLoc.h"
#include "llvm/MC/LaneBitmask.h" #include "llvm/MC/LaneBitmask.h"
#include "llvm/MC/MCDwarf.h"
#include "llvm/MC/MCRegisterInfo.h" #include "llvm/MC/MCRegisterInfo.h"
#include "llvm/Support/BranchProbability.h" #include "llvm/Support/BranchProbability.h"
#include <cassert> #include <cassert>
#include <cstdint> #include <cstdint>
#include <functional> #include <functional>
#include <iterator> #include <iterator>
#include <string> #include <string>
#include <vector> #include <vector>
▲ Show 20 Lines • Show All 718 Lines • ▼ Show 20 Linesprivate:
/// unless you know what you're doing, because it doesn't update Pred's /// unless you know what you're doing, because it doesn't update Pred's
/// successors list. Use Pred->addSuccessor instead. /// successors list. Use Pred->addSuccessor instead.
void addPredecessor(MachineBasicBlock *Pred); void addPredecessor(MachineBasicBlock *Pred);
/// Remove Pred as a predecessor of this MachineBasicBlock. Don't do this /// Remove Pred as a predecessor of this MachineBasicBlock. Don't do this
/// unless you know what you're doing, because it doesn't update Pred's /// unless you know what you're doing, because it doesn't update Pred's
/// successors list. Use Pred->removeSuccessor instead. /// successors list. Use Pred->removeSuccessor instead.
void removePredecessor(MachineBasicBlock *Pred); void removePredecessor(MachineBasicBlock *Pred);
// Value of cfa offset valid at basic block entry. It takes into account only
// the absolute offset that is set. It is adjusted with
// AdjustIncomingCFAOffset in CFIInfoVerifier.
int IncomingCFAOffset = -1;
// Value of cfa offset valid at basic block exit. It takes into account only
// the absolute offset that is set. It is adjusted with
// AdjustOutgoingCFAOffset in CFIInfoVerifier.
int OutgoingCFAOffset = -1;
// Value of cfa register valid at basic block entry.
unsigned IncomingCFARegister = 0;
// Value of cfa register valid at basic block exit.
unsigned OutgoingCFARegister = 0;
// If a block contains a def_cfa_offset or def_cfa directive.
bool DefOffset = false;
// If a block contains a def_cfa_register or def_cfa directive.
bool DefRegister = false;
// Amount used for adjusting the value of incoming cfa offset. This value
// represents the sum of adjust_cfa_offset directives in effect at this
// block's entry. It is added to IncomingCFAOffset in CFIInfoVerifier.
int AdjustIncomingCFAOffset = 0;
iterateeUnsubmitted
Not Done
ReplyInline Actions

I don't follow the comment about why these are necessary again.
Can you elaborate?

iteratee: I don't follow the comment about why these are necessary again. Can you elaborate?
violetavAuthorUnsubmitted
Not Done
ReplyInline Actions

I will try. So, let's say there is a function that looks like this:

  BB_0
    | 
  BB_1 <-----|
  /  \       |
BB_2 BB_3    |
       |     |
     BB_4    |
       |     |
     BB_5 ___|

and that the following is true:

  1. BB_1 has following cfi directives:

BB_1:

...
.cfi_adjust_cfa_offset 4
...
.cfi_adjust_cfa_offset -4
...
  1. BB_4 has the same:

BB_4:

...
.cfi_adjust_cfa_offset 4
...
.cfi_adjust_cfa_offset -4
...
  1. BB_1 is a successor of BB_5 (if it's not clear from the 'illustration', BB_0 has one successor, BB_1 has two, and BB_3,BB_4,BB_5 each have one).
  1. For this example, first the cfi_adjust_cfa directives with positive values are inserted (in both BB_1 and BB_4) and then the ones with negative values.

What happens here with the previous approach is that, although BB_1 and BB_4 do not change cfa offset (their incoming and outgoing offsets are the same), they and their successors end up having the wrong offset set.

This was the way that in/out offset was set to successors in the previous approach (from updateCFIInfoSucc()):

// the difference remains the same, calculate it by using previous in/out offset values
int AdjustAmount = Succ->getOutgoingCFAOffset() - Succ->getIncomingCFAOffset();
// set outgoing offset to be sum of the outgoing offset of the predecessor and the amount that this successor adjusts the offset itself (with adjust_cfa_directives)
Succ->setOutgoingCFAOffset(CurrSucc->getOutgoingCFAOffset() + AdjustAmount);
// set incoming offset to be the outgoing offset of the predecessor
Succ->setIncomingCFAOffset(CurrSucc->getOutgoingCFAOffset());

Let's say that all blocks have in/out offset set to zero at the beginning (just to simplify the numbers).
When first 'adjust_cfa_offset 4' is inserted in BB_1, after updating the successors, the in/out cfa offsets of BBs in the function look like this:

BB_0, in 0, out 0
BB_1, in 0, out 4
BB_2, in 4, out 4
BB_3, in 4, out 4
BB_4, in 4, out 4
BB_5, in 4, out 4

Next, when 'adjust_cfa_offset 4' is inserted in BB_4:

BB_0, in 0, out 0
BB_1, in 8, out 12
BB_2, in 12, out 12
BB_3, in 12, out 12
BB_4, in 4, out 8
BB_5, in 8, out 8

Next, when 'adjust_cfa_offset -4' is inserted in BB_1:

BB_0, in 0, out 0
BB_1, in 8, out 8
BB_2, in 8, out 8
BB_3, in 8, out 8
BB_4, in 8, out 12
BB_5, in 12, out 12

Next, when 'adjust_cfa_offset -4' is inserted in BB_4:

BB_0, in 0, out 0
BB_1, in 8, out 8
BB_2, in 8, out 8
BB_3, in 8, out 8
BB_4, in 8, out 8
BB_5, in 8, out 8

And then you end up having wrong in/out cfa offset for blocks 1-5 because the adjustments are contained in OutgoingCFAOffset which is the value that gets propagated further. That is what is propagated to the successors, instead of the increment/decrement from adjust directives. This way of calculating in/out cfa offset values produces wrong results for the case where you have a cycle (like in the example above).

What is changed now is that each 'adjust_cfa_offset' directive is propagated to the successors when it's inserted and updates their incoming and outgoing adjust values appropriately. In the end, before verifying CFI info and inserting additional CFI instructions if needed, these values are added to incoming and outgoing cfa offset (that now contain only absolute offsets that are set).

violetav: I will try. So, let's say there is a function that looks like this: BB_0 | BB_1…
// Amount used for adjusting the value of outgoing cfa offset. This value
// represents the sum of adjust_cfa_offset directives in effect at this
// block's exit. It is added to OutgoingCFAOffset in CFIInfoVerifier.
int AdjustOutgoingCFAOffset = 0;
public:
int getIncomingCFAOffset() { return IncomingCFAOffset; }
void setIncomingCFAOffset(int Offset) { IncomingCFAOffset = Offset; }
int getOutgoingCFAOffset() { return OutgoingCFAOffset; }
void setOutgoingCFAOffset(int Offset) { OutgoingCFAOffset = Offset; }
unsigned getIncomingCFARegister() { return IncomingCFARegister; }
void setIncomingCFARegister(unsigned Register) {
IncomingCFARegister = Register;
}
unsigned getOutgoingCFARegister() { return OutgoingCFARegister; }
void setOutgoingCFARegister(unsigned Register) {
OutgoingCFARegister = Register;
}
bool hasDefOffset() { return DefOffset; }
bool hasDefRegister() { return DefRegister; }
void setDefOffset(bool SetsOffset) { DefOffset = SetsOffset; }
void setDefRegister(bool SetsRegister) { DefRegister = SetsRegister; }
int getAdjustIncomingCFAOffset() { return AdjustIncomingCFAOffset; }
void setAdjustIncomingCFAOffset(int Offset) {
AdjustIncomingCFAOffset = Offset;
}
int getAdjustOutgoingCFAOffset() { return AdjustOutgoingCFAOffset; }
void setAdjustOutgoingCFAOffset(int Offset) {
AdjustOutgoingCFAOffset = Offset;
}
// Increment values for adjusting cfa offset by AdjustOffset for all
// successors of this block.
void updateAdjustOffsetSucc(int AdjustOffset);
// Update the outgoing cfa offset, adjustment and register for this block
// based on the CFI instruction inserted at Pos.
void updateCFIInfo(MachineBasicBlock::iterator Pos);
// Update the cfa offset and register values for all successors of this block.
void updateCFIInfoSucc();
// Recalculate outgoing cfa offset, adjustment and register. Use existing
// incoming offset, adjustment and register values if UseExistingIncoming is
// set to true. If it is false, use new values passed as arguments.
void recalculateCFIInfo(bool UseExistingIncoming, int NewIncomingOffset = -1,
unsigned NewIncomingRegister = 0,
int NewAdjustIncomingOffset = 0);
// Update outgoing cfa offset, adjustment and register of the block after it
// is merged with MBB.
void mergeCFIInfo(MachineBasicBlock *MBB);
}; };
raw_ostream& operator<<(raw_ostream &OS, const MachineBasicBlock &MBB); raw_ostream& operator<<(raw_ostream &OS, const MachineBasicBlock &MBB);
// This is useful when building IndexedMaps keyed on basic block pointers. // This is useful when building IndexedMaps keyed on basic block pointers.
struct MBB2NumberFunctor : struct MBB2NumberFunctor :
public std::unary_function<const MachineBasicBlock*, unsigned> { public std::unary_function<const MachineBasicBlock*, unsigned> {
unsigned operator()(const MachineBasicBlock *MBB) const { unsigned operator()(const MachineBasicBlock *MBB) const {
▲ Show 20 Lines • Show All 109 Lines • Show Last 20 Lines