IR: RF_IgnoreMissingValues => RF_IgnoreMissingLocals, NFC

Clarify what this RemapFlag actually means.

  - Change the flag name to match its intended behaviour.
  - Clearly document that it's not supposed to affect globals.
  - Add a host of FIXMEs to indicate how to fix the behaviour to match
    the intent of the flag.

RF_IgnoreMissingLocals should only affect the behaviour of
RemapInstruction for function-local operands; namely, for operands of
type Argument, Instruction, and BasicBlock.  Currently, it is *only*
passed into RemapInstruction calls (and the transitive MapValue calls
that it makes).

When I split Metadata from Value I didn't understand the flag, and I
used it in a bunch of places for "global" metadata.

This commit doesn't have any functionality change, but prepares to
cleanup MapMetadata and MapValue.

llvm-svn: 265628
This commit is contained in:
Duncan P. N. Exon Smith 2016-04-07 00:26:43 +00:00
parent be98b33b17
commit 5f260975e3
10 changed files with 45 additions and 18 deletions

View File

@ -67,17 +67,22 @@ enum RemapFlags {
/// values like functions and global metadata.
RF_NoModuleLevelChanges = 1,
/// If this flag is set, the remapper ignores entries that are not in the
/// If this flag is set, the remapper ignores missing function-local entries
/// (Argument, Instruction, BasicBlock) that are not in the
/// value map. If it is unset, it aborts if an operand is asked to be
/// remapped which doesn't exist in the mapping.
RF_IgnoreMissingEntries = 2,
///
/// There are no such assertions in MapValue(), whose result should be
/// essentially unchanged by this flag. This only changes the assertion
/// behaviour in RemapInstruction().
RF_IgnoreMissingLocals = 2,
/// Instruct the remapper to move distinct metadata instead of duplicating it
/// when there are module-level changes.
RF_MoveDistinctMDs = 4,
/// Any global values not in value map are mapped to null instead of mapping
/// to self. Illegal if RF_IgnoreMissingEntries is also set.
/// to self. Illegal if RF_IgnoreMissingLocals is also set.
RF_NullMapMissingGlobalValues = 8,
};
@ -85,6 +90,18 @@ static inline RemapFlags operator|(RemapFlags LHS, RemapFlags RHS) {
return RemapFlags(unsigned(LHS) | unsigned(RHS));
}
/// Look up or compute a value in the value map.
///
/// Return a mapped value for a function-local value (Argument, Instruction,
/// BasicBlock), or compute and memoize a value for a Constant.
///
/// 1. If \c V is in VM, return the result.
/// 2. Else if \c V can be materialized with \c Materializer, do so, memoize
/// it in \c VM, and return it.
/// 3. Else if \c V is a function-local value, return nullptr.
/// 4. Else if \c V is a \a GlobalValue, return \c nullptr or \c V depending
/// on \a RF_NullMapMissingGlobalValues.
/// 5. Else, Compute the equivalent constant, and return it.
Value *MapValue(const Value *V, ValueToValueMapTy &VM,
RemapFlags Flags = RF_None,
ValueMapTypeRemapper *TypeMapper = nullptr,

View File

@ -787,7 +787,7 @@ void WinEHPrepare::cloneCommonBlocks(Function &F) {
// Loop over all instructions, fixing each one as we find it...
for (Instruction &I : *BB)
RemapInstruction(&I, VMap,
RF_IgnoreMissingEntries | RF_NoModuleLevelChanges);
RF_IgnoreMissingLocals | RF_NoModuleLevelChanges);
// Catchrets targeting cloned blocks need to be updated separately from
// the loop above because they are not in the current funclet.

View File

@ -997,7 +997,7 @@ bool IRLinker::linkFunctionBody(Function &Dst, Function &Src) {
A.mutateType(TypeMap.get(A.getType()));
for (BasicBlock &BB : Dst)
for (Instruction &I : BB)
RemapInstruction(&I, ValueMap, RF_IgnoreMissingEntries | ValueMapperFlags,
RemapInstruction(&I, ValueMap, RF_IgnoreMissingLocals | ValueMapperFlags,
&TypeMap, &GValMaterializer);
return false;

View File

@ -939,7 +939,7 @@ void LoopConstrainer::cloneLoop(LoopConstrainer::ClonedLoop &Result,
for (Instruction &I : *ClonedBB)
RemapInstruction(&I, Result.Map,
RF_NoModuleLevelChanges | RF_IgnoreMissingEntries);
RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
// Exit blocks will now have one more predecessor and their PHI nodes need
// to be edited to reflect that. No phi nodes need to be introduced because

View File

@ -244,7 +244,7 @@ static bool rotateLoop(Loop *L, unsigned MaxHeaderSize, LoopInfo *LI,
// Eagerly remap the operands of the instruction.
RemapInstruction(C, ValueMap,
RF_NoModuleLevelChanges|RF_IgnoreMissingEntries);
RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
// With the operands remapped, see if the instruction constant folds or is
// otherwise simplifyable. This commonly occurs because the entry from PHI

View File

@ -1068,7 +1068,7 @@ void LoopUnswitch::UnswitchNontrivialCondition(Value *LIC, Constant *Val,
for (BasicBlock::iterator I = NewBlocks[i]->begin(),
E = NewBlocks[i]->end(); I != E; ++I)
RemapInstruction(&*I, VMap,
RF_NoModuleLevelChanges | RF_IgnoreMissingEntries);
RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
// Rewrite the original preheader to select between versions of the loop.
BranchInst *OldBR = cast<BranchInst>(loopPreheader->getTerminator());

View File

@ -694,7 +694,7 @@ void llvm::remapInstructionsInBlocks(
for (auto *BB : Blocks)
for (auto &Inst : *BB)
RemapInstruction(&Inst, VMap,
RF_NoModuleLevelChanges | RF_IgnoreMissingEntries);
RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
}
/// \brief Clones a loop \p OrigLoop. Returns the loop and the blocks in \p

View File

@ -636,7 +636,7 @@ bool llvm::UnrollRuntimeLoopRemainder(Loop *L, unsigned Count,
for (BasicBlock *BB : NewBlocks) {
for (Instruction &I : *BB) {
RemapInstruction(&I, VMap,
RF_NoModuleLevelChanges | RF_IgnoreMissingEntries);
RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
}
}

View File

@ -2236,7 +2236,7 @@ bool llvm::FoldBranchToCommonDest(BranchInst *BI, unsigned BonusInstThreshold) {
continue;
Instruction *NewBonusInst = BonusInst->clone();
RemapInstruction(NewBonusInst, VMap,
RF_NoModuleLevelChanges | RF_IgnoreMissingEntries);
RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
VMap[&*BonusInst] = NewBonusInst;
// If we moved a load, we cannot any longer claim any knowledge about
@ -2255,7 +2255,7 @@ bool llvm::FoldBranchToCommonDest(BranchInst *BI, unsigned BonusInstThreshold) {
// two conditions together.
Instruction *New = Cond->clone();
RemapInstruction(New, VMap,
RF_NoModuleLevelChanges | RF_IgnoreMissingEntries);
RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
PredBlock->getInstList().insert(PBI->getIterator(), New);
New->takeName(Cond);
Cond->setName(New->getName() + ".old");

View File

@ -274,9 +274,11 @@ Value *Mapper::mapValue(const Value *V) {
// are using the identity mapping.
if (isa<GlobalValue>(V)) {
if (Flags & RF_NullMapMissingGlobalValues) {
assert(!(Flags & RF_IgnoreMissingEntries) &&
// FIXME: Remove this assertion. RF_IgnoreMissingLocals is unrelated to
// RF_NullMapMissingGlobalValues.
assert(!(Flags & RF_IgnoreMissingLocals) &&
"Illegal to specify both RF_NullMapMissingGlobalValues and "
"RF_IgnoreMissingEntries");
"RF_IgnoreMissingLocals");
return nullptr;
}
return VM[V] = const_cast<Value*>(V);
@ -303,8 +305,11 @@ Value *Mapper::mapValue(const Value *V) {
if (!isa<LocalAsMetadata>(MD) && (Flags & RF_NoModuleLevelChanges))
return VM[V] = const_cast<Value *>(V);
// FIXME: be consistent with function-local values for LocalAsMetadata by
// returning nullptr when LocalAsMetadata is missing. Adding a mapping is
// expensive.
auto *MappedMD = mapMetadata(MD);
if (MD == MappedMD || (!MappedMD && (Flags & RF_IgnoreMissingEntries)))
if (MD == MappedMD || (!MappedMD && (Flags & RF_IgnoreMissingLocals)))
return VM[V] = const_cast<Value *>(V);
return VM[V] = MetadataAsValue::get(V->getContext(), MappedMD);
@ -628,14 +633,17 @@ Optional<Metadata *> Mapper::mapSimpleMetadata(const Metadata *MD) {
if ((Flags & RF_NoModuleLevelChanges))
return mapToSelf(MD);
// FIXME: Assert that this is not LocalAsMetadata. It should be handled
// elsewhere.
if (const auto *VMD = dyn_cast<ValueAsMetadata>(MD)) {
// Disallow recursion into metadata mapping through mapValue.
VM.disableMapMetadata();
Value *MappedV = mapValue(VMD->getValue());
VM.enableMapMetadata();
// FIXME: Always use "ignore" behaviour. There should only be globals here.
if (VMD->getValue() == MappedV ||
(!MappedV && (Flags & RF_IgnoreMissingEntries)))
(!MappedV && (Flags & RF_IgnoreMissingLocals)))
return mapToSelf(MD);
return mapToMetadata(MD, MappedV ? ValueAsMetadata::get(MappedV) : nullptr);
@ -658,6 +666,8 @@ Metadata *llvm::MapMetadata(const Metadata *MD, ValueToValueMapTy &VM,
}
Metadata *Mapper::mapMetadata(const Metadata *MD) {
// FIXME: First check for and deal with LocalAsMetadata, so that
// mapSimpleMetadata() doesn't need to deal with it.
if (Optional<Metadata *> NewMD = mapSimpleMetadata(MD))
return *NewMD;
@ -703,7 +713,7 @@ void llvm::RemapInstruction(Instruction *I, ValueToValueMapTy &VMap,
if (V)
*op = V;
else
assert((Flags & RF_IgnoreMissingEntries) &&
assert((Flags & RF_IgnoreMissingLocals) &&
"Referenced value not in value map!");
}
@ -715,7 +725,7 @@ void llvm::RemapInstruction(Instruction *I, ValueToValueMapTy &VMap,
if (V)
PN->setIncomingBlock(i, cast<BasicBlock>(V));
else
assert((Flags & RF_IgnoreMissingEntries) &&
assert((Flags & RF_IgnoreMissingLocals) &&
"Referenced block not in value map!");
}
}