Skip to content
This repository has been archived by the owner on Apr 23, 2020. It is now read-only.

Commit

Permalink
[x86] Introduce a pass to begin more systematically fixing PR36028 an…
Browse files Browse the repository at this point in the history
…d similar issues.

The key idea is to lower COPY nodes populating EFLAGS by scanning the
uses of EFLAGS and introducing dedicated code to preserve the necessary
state in a GPR. In the vast majority of cases, these uses are cmovCC and
jCC instructions. For such cases, we can very easily save and restore
the necessary information by simply inserting a setCC into a GPR where
the original flags are live, and then testing that GPR directly to feed
the cmov or conditional branch.

However, things are a bit more tricky if arithmetic is using the flags.
This patch handles the vast majority of cases that seem to come up in
practice: adc, adcx, adox, rcl, and rcr; all without taking advantage of
partially preserved EFLAGS as LLVM doesn't currently model that at all.

There are a large number of operations that techinaclly observe EFLAGS
currently but shouldn't in this case -- they typically are using DF.
Currently, they will not be handled by this approach. However, I have
never seen this issue come up in practice. It is already pretty rare to
have these patterns come up in practical code with LLVM. I had to resort
to writing MIR tests to cover most of the logic in this pass already.
I suspect even with its current amount of coverage of arithmetic users
of EFLAGS it will be a significant improvement over the current use of
pushf/popf. It will also produce substantially faster code in most of
the common patterns.

This patch also removes all of the old lowering for EFLAGS copies, and
the hack that forced us to use a frame pointer when EFLAGS copies were
found anywhere in a function so that the dynamic stack adjustment wasn't
a problem. None of this is needed as we now lower all of these copies
directly in MI and without require stack adjustments.

Lots of thanks to Reid who came up with several aspects of this
approach, and Craig who helped me work out a couple of things tripping
me up while working on this.

Differential Revision: https://reviews.llvm.org/D45146

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@329657 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
chandlerc committed Apr 10, 2018
1 parent a379fe2 commit 21a0c18
Show file tree
Hide file tree
Showing 21 changed files with 4,471 additions and 3,827 deletions.
7 changes: 7 additions & 0 deletions include/llvm/CodeGen/MachineBasicBlock.h
Expand Up @@ -457,6 +457,13 @@ class MachineBasicBlock
/// Replace successor OLD with NEW and update probability info.
void replaceSuccessor(MachineBasicBlock *Old, MachineBasicBlock *New);

/// Copy a successor (and any probability info) from original block to this
/// block's. Uses an iterator into the original blocks successors.
///
/// This is useful when doing a partial clone of successors. Afterward, the
/// probabilities may need to be normalized.
void copySuccessor(MachineBasicBlock *Orig, succ_iterator I);

/// Transfers all the successors from MBB to this machine basic block (i.e.,
/// copies all the successors FromMBB and remove all the successors from
/// FromMBB).
Expand Down
8 changes: 8 additions & 0 deletions lib/CodeGen/MachineBasicBlock.cpp
Expand Up @@ -720,6 +720,14 @@ void MachineBasicBlock::replaceSuccessor(MachineBasicBlock *Old,
removeSuccessor(OldI);
}

void MachineBasicBlock::copySuccessor(MachineBasicBlock *Orig,
succ_iterator I) {
if (Orig->Probs.empty())
addSuccessor(*I, Orig->getSuccProbability(I));
else
addSuccessorWithoutProb(*I);
}

void MachineBasicBlock::addPredecessor(MachineBasicBlock *Pred) {
Predecessors.push_back(Pred);
}
Expand Down
1 change: 1 addition & 0 deletions lib/Target/X86/CMakeLists.txt
Expand Up @@ -34,6 +34,7 @@ set(sources
X86FixupLEAs.cpp
X86AvoidStoreForwardingBlocks.cpp
X86FixupSetCC.cpp
X86FlagsCopyLowering.cpp
X86FloatingPoint.cpp
X86FrameLowering.cpp
X86InstructionSelector.cpp
Expand Down
3 changes: 3 additions & 0 deletions lib/Target/X86/X86.h
Expand Up @@ -78,6 +78,9 @@ FunctionPass *createX86FixupSetCC();
/// Return a pass that avoids creating store forward block issues in the hardware.
FunctionPass *createX86AvoidStoreForwardingBlocks();

/// Return a pass that lowers EFLAGS copy pseudo instructions.
FunctionPass *createX86FlagsCopyLoweringPass();

/// Return a pass that expands WinAlloca pseudo-instructions.
FunctionPass *createX86WinAllocaExpander();

Expand Down

0 comments on commit 21a0c18

Please sign in to comment.