From 198f71f698cddbb0c3b268f39d10c3081cbfd86d Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Sun, 15 Jul 2018 23:46:36 +0000 Subject: [PATCH] [x86/SLH] Extract one of the bits of logic to its own function. NFC. This is just a refactoring to start cleaning up the code here and make it more readable and approachable. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@337138 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../X86/X86SpeculativeLoadHardening.cpp | 91 ++++++++++--------- 1 file changed, 48 insertions(+), 43 deletions(-) diff --git a/lib/Target/X86/X86SpeculativeLoadHardening.cpp b/lib/Target/X86/X86SpeculativeLoadHardening.cpp index f5933cbc8ca..e0974f0eb5b 100644 --- a/lib/Target/X86/X86SpeculativeLoadHardening.cpp +++ b/lib/Target/X86/X86SpeculativeLoadHardening.cpp @@ -283,6 +283,53 @@ static MachineBasicBlock &splitEdge(MachineBasicBlock &MBB, return NewMBB; } +/// Removing duplicate PHI operands to leave the PHI in a canonical and +/// predictable form. +/// +/// FIXME: It's really frustrating that we have to do this, but SSA-form in MIR +/// isn't what you might expect. We may have multiple entries in PHI nodes for +/// a single predecessor. This makes CFG-updating extremely complex, so here we +/// simplify all PHI nodes to a model even simpler than the IR's model: exactly +/// one entry per predecessor, regardless of how many edges there are. +static void canonicalizePHIOperands(MachineFunction &MF) { + SmallPtrSet Preds; + SmallVector DupIndices; + for (auto &MBB : MF) + for (auto &MI : MBB) { + if (!MI.isPHI()) + break; + + // First we scan the operands of the PHI looking for duplicate entries + // a particular predecessor. We retain the operand index of each duplicate + // entry found. + for (int OpIdx = 1, NumOps = MI.getNumOperands(); OpIdx < NumOps; + OpIdx += 2) + if (!Preds.insert(MI.getOperand(OpIdx + 1).getMBB()).second) + DupIndices.push_back(OpIdx); + + // Now walk the duplicate indices, removing both the block and value. Note + // that these are stored as a vector making this element-wise removal + // :w + // potentially quadratic. + // + // FIXME: It is really frustrating that we have to use a quadratic + // removal algorithm here. There should be a better way, but the use-def + // updates required make that impossible using the public API. + // + // Note that we have to process these backwards so that we don't + // invalidate other indices with each removal. + while (!DupIndices.empty()) { + int OpIdx = DupIndices.pop_back_val(); + // Remove both the block and value operand, again in reverse order to + // preserve indices. + MI.RemoveOperand(OpIdx + 1); + MI.RemoveOperand(OpIdx); + } + + Preds.clear(); + } +} + bool X86SpeculativeLoadHardeningPass::runOnMachineFunction( MachineFunction &MF) { LLVM_DEBUG(dbgs() << "********** " << getPassName() << " : " << MF.getName() @@ -401,49 +448,7 @@ bool X86SpeculativeLoadHardeningPass::runOnMachineFunction( // We're going to need to trace predicate state throughout the function's // CFG. Prepare for this by setting up our initial state of PHIs with unique // predecessor entries and all the initial predicate state. - - // FIXME: It's really frustrating that we have to do this, but SSA-form in - // MIR isn't what you might expect. We may have multiple entries in PHI nodes - // for a single predecessor. This makes CFG-updating extremely complex, so - // here we simplify all PHI nodes to a model even simpler than the IR's - // model: exactly one entry per predecessor, regardless of how many edges - // there are. - SmallPtrSet Preds; - SmallVector DupIndices; - for (auto &MBB : MF) - for (auto &MI : MBB) { - if (!MI.isPHI()) - break; - - // First we scan the operands of the PHI looking for duplicate entries - // a particular predecessor. We retain the operand index of each duplicate - // entry found. - for (int OpIdx = 1, NumOps = MI.getNumOperands(); OpIdx < NumOps; - OpIdx += 2) - if (!Preds.insert(MI.getOperand(OpIdx + 1).getMBB()).second) - DupIndices.push_back(OpIdx); - - // Now walk the duplicate indices, removing both the block and value. Note - // that these are stored as a vector making this element-wise removal - // :w - // potentially quadratic. - // - // FIXME: It is really frustrating that we have to use a quadratic - // removal algorithm here. There should be a better way, but the use-def - // updates required make that impossible using the public API. - // - // Note that we have to process these backwards so that we don't - // invalidate other indices with each removal. - while (!DupIndices.empty()) { - int OpIdx = DupIndices.pop_back_val(); - // Remove both the block and value operand, again in reverse order to - // preserve indices. - MI.RemoveOperand(OpIdx + 1); - MI.RemoveOperand(OpIdx); - } - - Preds.clear(); - } + canonicalizePHIOperands(MF); // Track the updated values in an SSA updater to rewrite into SSA form at the // end. -- 2.50.1