[llvm] r265628 - IR: RF_IgnoreMissingValues => RF_IgnoreMissingLocals, NFC

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 6 17:26:45 PDT 2016


Author: dexonsmith
Date: Wed Apr  6 19:26:43 2016
New Revision: 265628

URL: http://llvm.org/viewvc/llvm-project?rev=265628&view=rev
Log:
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.

Modified:
    llvm/trunk/include/llvm/Transforms/Utils/ValueMapper.h
    llvm/trunk/lib/CodeGen/WinEHPrepare.cpp
    llvm/trunk/lib/Linker/IRMover.cpp
    llvm/trunk/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
    llvm/trunk/lib/Transforms/Scalar/LoopRotation.cpp
    llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp
    llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp
    llvm/trunk/lib/Transforms/Utils/LoopUnrollRuntime.cpp
    llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
    llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp

Modified: llvm/trunk/include/llvm/Transforms/Utils/ValueMapper.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/ValueMapper.h?rev=265628&r1=265627&r2=265628&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/Utils/ValueMapper.h (original)
+++ llvm/trunk/include/llvm/Transforms/Utils/ValueMapper.h Wed Apr  6 19:26:43 2016
@@ -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|(Remap
   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,

Modified: llvm/trunk/lib/CodeGen/WinEHPrepare.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/WinEHPrepare.cpp?rev=265628&r1=265627&r2=265628&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/WinEHPrepare.cpp (original)
+++ llvm/trunk/lib/CodeGen/WinEHPrepare.cpp Wed Apr  6 19:26:43 2016
@@ -787,7 +787,7 @@ void WinEHPrepare::cloneCommonBlocks(Fun
       // 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.

Modified: llvm/trunk/lib/Linker/IRMover.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/IRMover.cpp?rev=265628&r1=265627&r2=265628&view=diff
==============================================================================
--- llvm/trunk/lib/Linker/IRMover.cpp (original)
+++ llvm/trunk/lib/Linker/IRMover.cpp Wed Apr  6 19:26:43 2016
@@ -997,7 +997,7 @@ bool IRLinker::linkFunctionBody(Function
     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;

Modified: llvm/trunk/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp?rev=265628&r1=265627&r2=265628&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp Wed Apr  6 19:26:43 2016
@@ -939,7 +939,7 @@ void LoopConstrainer::cloneLoop(LoopCons
 
     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

Modified: llvm/trunk/lib/Transforms/Scalar/LoopRotation.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoopRotation.cpp?rev=265628&r1=265627&r2=265628&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/LoopRotation.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/LoopRotation.cpp Wed Apr  6 19:26:43 2016
@@ -244,7 +244,7 @@ static bool rotateLoop(Loop *L, unsigned
 
     // 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

Modified: llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp?rev=265628&r1=265627&r2=265628&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp Wed Apr  6 19:26:43 2016
@@ -1068,7 +1068,7 @@ void LoopUnswitch::UnswitchNontrivialCon
     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());

Modified: llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp?rev=265628&r1=265627&r2=265628&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp Wed Apr  6 19:26:43 2016
@@ -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

Modified: llvm/trunk/lib/Transforms/Utils/LoopUnrollRuntime.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/LoopUnrollRuntime.cpp?rev=265628&r1=265627&r2=265628&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/LoopUnrollRuntime.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/LoopUnrollRuntime.cpp Wed Apr  6 19:26:43 2016
@@ -636,7 +636,7 @@ bool llvm::UnrollRuntimeLoopRemainder(Lo
   for (BasicBlock *BB : NewBlocks) {
     for (Instruction &I : *BB) {
       RemapInstruction(&I, VMap,
-                       RF_NoModuleLevelChanges | RF_IgnoreMissingEntries);
+                       RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
     }
   }
 

Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=265628&r1=265627&r2=265628&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Wed Apr  6 19:26:43 2016
@@ -2236,7 +2236,7 @@ bool llvm::FoldBranchToCommonDest(Branch
         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(Branch
     // 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");

Modified: llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp?rev=265628&r1=265627&r2=265628&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp Wed Apr  6 19:26:43 2016
@@ -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::mapSimpleMe
     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 Metada
 }
 
 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
     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
       if (V)
         PN->setIncomingBlock(i, cast<BasicBlock>(V));
       else
-        assert((Flags & RF_IgnoreMissingEntries) &&
+        assert((Flags & RF_IgnoreMissingLocals) &&
                "Referenced block not in value map!");
     }
   }




More information about the llvm-commits mailing list