[llvm] r265631 - ValueMapper: Make LocalAsMetadata match function-local Values

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 6 19:22:35 PDT 2016


Reverted in r265637 due to bot failures:
http://lab.llvm.org:8011/builders/clang-cmake-aarch64-42vma/builds/7272
http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/3256
http://lab.llvm.org:8080/green/job/clang-stage1-configure-RA_build/18973

The last one was in compiler-rt on Darwin, so I should be able to reproduce.

> On 2016-Apr-06, at 18:08, Duncan P. N. Exon Smith via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: dexonsmith
> Date: Wed Apr  6 20:08:39 2016
> New Revision: 265631
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=265631&view=rev
> Log:
> ValueMapper: Make LocalAsMetadata match function-local Values
> 
> Start treating LocalAsMetadata similarly to function-local members of
> the Value hierarchy in MapValue and MapMetadata.
> 
>  - Don't memoize them.
>  - Return nullptr if they are missing.
> 
> This also cleans up ConstantAsMetadata to stop listening to the
> RF_IgnoreMissingLocals flag.
> 
> Modified:
>    llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
>    llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp
> 
> Modified: llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp?rev=265631&r1=265630&r2=265631&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp Wed Apr  6 20:08:39 2016
> @@ -86,6 +86,9 @@ public:
>   /// (not an MDNode, or MDNode::isResolved() returns true).
>   Metadata *mapMetadata(const Metadata *MD);
> 
> +  // Map LocalAsMetadata, which never gets memoized.
> +  Metadata *mapLocalAsMetadata(const LocalAsMetadata &LAM);
> +
> private:
>   Value *mapBlockAddress(const BlockAddress &BA);
> 
> @@ -300,18 +303,27 @@ Value *Mapper::mapValue(const Value *V)
> 
>   if (const auto *MDV = dyn_cast<MetadataAsValue>(V)) {
>     const Metadata *MD = MDV->getMetadata();
> +
> +    // Locals shouldn't be memoized.  Return nullptr if mapLocalAsMetadata()
> +    // returns nullptr; otherwise bridge back to the Value hierarchy.
> +    if (auto *LAM = dyn_cast<LocalAsMetadata>(MD)) {
> +      if (auto *MappedMD = mapLocalAsMetadata(*LAM)) {
> +        if (LAM == MappedMD)
> +          return const_cast<Value *>(V);
> +        return MetadataAsValue::get(V->getContext(), MappedMD);
> +      }
> +      return nullptr;
> +    }
> +
>     // If this is a module-level metadata and we know that nothing at the module
>     // level is changing, then use an identity mapping.
> -    if (!isa<LocalAsMetadata>(MD) && (Flags & RF_NoModuleLevelChanges))
> +    if (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.
> +    // Map the metadata and turn it into a value.
>     auto *MappedMD = mapMetadata(MD);
> -    if (MD == MappedMD || (!MappedMD && (Flags & RF_IgnoreMissingLocals)))
> +    if (MD == MappedMD)
>       return VM[V] = const_cast<Value *>(V);
> -
>     return VM[V] = MetadataAsValue::get(V->getContext(), MappedMD);
>   }
> 
> @@ -629,21 +641,16 @@ Optional<Metadata *> Mapper::mapSimpleMe
>   if (isa<MDString>(MD))
>     return mapToSelf(MD);
> 
> -  if (isa<ConstantAsMetadata>(MD))
> +  if (auto *CMD = dyn_cast<ConstantAsMetadata>(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());
> +    Value *MappedV = mapValue(CMD->getValue());
>     VM.enableMapMetadata();
> 
> -    // FIXME: Always use "ignore" behaviour.  There should only be globals here.
> -    if (VMD->getValue() == MappedV ||
> -        (!MappedV && (Flags & RF_IgnoreMissingLocals)))
> +    if (CMD->getValue() == MappedV)
>       return mapToSelf(MD);
> 
>     return mapToMetadata(MD, MappedV ? ValueAsMetadata::get(MappedV) : nullptr);
> @@ -665,9 +672,24 @@ Metadata *llvm::MapMetadata(const Metada
>   return Mapper(VM, Flags, TypeMapper, Materializer).mapMetadata(MD);
> }
> 
> +Metadata *Mapper::mapLocalAsMetadata(const LocalAsMetadata &LAM) {
> +  if (Optional<Metadata *> NewMD = VM.getMappedMD(&LAM))
> +    return *NewMD;
> +
> +  // Lookup the mapping for the value itself, and return the appropriate
> +  // metadata.
> +  if (Value *V = mapValue(LAM.getValue())) {
> +    if (V == LAM.getValue())
> +      return const_cast<LocalAsMetadata *>(&LAM);
> +    return ValueAsMetadata::get(V);
> +  }
> +  return nullptr;
> +}
> +
> 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 (auto *LAM = dyn_cast<LocalAsMetadata>(MD))
> +    return mapLocalAsMetadata(*LAM);
> +
>   if (Optional<Metadata *> NewMD = mapSimpleMetadata(MD))
>     return *NewMD;
> 
> 
> Modified: llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp?rev=265631&r1=265630&r2=265631&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp (original)
> +++ llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp Wed Apr  6 20:08:39 2016
> @@ -7,6 +7,7 @@
> //
> //===----------------------------------------------------------------------===//
> 
> +#include "llvm/IR/Function.h"
> #include "llvm/IR/LLVMContext.h"
> #include "llvm/IR/Metadata.h"
> #include "llvm/Transforms/Utils/ValueMapper.h"
> @@ -126,4 +127,86 @@ TEST(ValueMapperTest, MapMetadataSeededW
>   EXPECT_EQ(nullptr, MapMetadata(D, VM, RF_None));
> }
> 
> +TEST(ValueMapperTest, MapMetadataLocalAsMetadata) {
> +  LLVMContext C;
> +  FunctionType *FTy =
> +      FunctionType::get(Type::getVoidTy(C), Type::getInt8Ty(C), false);
> +  std::unique_ptr<Function> F(
> +      Function::Create(FTy, GlobalValue::ExternalLinkage, "F"));
> +  Argument &A = *F->arg_begin();
> +
> +  auto *LAM = LocalAsMetadata::get(&A);
> +  ValueToValueMapTy VM;
> +  EXPECT_EQ(nullptr, MapMetadata(LAM, VM));
> +  EXPECT_EQ(nullptr, MapMetadata(LAM, VM, RF_IgnoreMissingLocals));
> +  EXPECT_EQ(None, VM.getMappedMD(LAM));
> +
> +  VM.MD()[LAM].reset(LAM);
> +  EXPECT_EQ(LAM, MapMetadata(LAM, VM));
> +  EXPECT_EQ(LAM, MapMetadata(LAM, VM, RF_IgnoreMissingLocals));
> +
> +  auto *N = MDNode::get(C, None);
> +  VM.MD()[LAM].reset(N);
> +  EXPECT_EQ(N, MapMetadata(LAM, VM));
> +  EXPECT_EQ(N, MapMetadata(LAM, VM, RF_IgnoreMissingLocals));
> +}
> +
> +TEST(ValueMapperTest, MapMetadataConstantAsMetadata) {
> +  LLVMContext C;
> +  FunctionType *FTy =
> +      FunctionType::get(Type::getVoidTy(C), Type::getInt8Ty(C), false);
> +  std::unique_ptr<Function> F(
> +      Function::Create(FTy, GlobalValue::ExternalLinkage, "F"));
> +
> +  auto *CAM = ConstantAsMetadata::get(F.get());
> +  {
> +    ValueToValueMapTy VM;
> +    EXPECT_EQ(CAM, MapMetadata(CAM, VM));
> +    EXPECT_TRUE(VM.MD().count(CAM));
> +    VM.MD().erase(CAM);
> +    EXPECT_EQ(CAM, MapMetadata(CAM, VM, RF_IgnoreMissingLocals));
> +    EXPECT_TRUE(VM.MD().count(CAM));
> +
> +    auto *N = MDNode::get(C, None);
> +    VM.MD()[CAM].reset(N);
> +    EXPECT_EQ(N, MapMetadata(CAM, VM));
> +    EXPECT_EQ(N, MapMetadata(CAM, VM, RF_IgnoreMissingLocals));
> +  }
> +
> +  std::unique_ptr<Function> F2(
> +      Function::Create(FTy, GlobalValue::ExternalLinkage, "F2"));
> +  ValueToValueMapTy VM;
> +  VM[F.get()] = F2.get();
> +  auto *F2MD = MapMetadata(CAM, VM);
> +  EXPECT_TRUE(VM.MD().count(CAM));
> +  EXPECT_TRUE(F2MD);
> +  EXPECT_EQ(F2.get(), cast<ConstantAsMetadata>(F2MD)->getValue());
> +}
> +
> +TEST(ValueMapperTest, MapValueLocalAsMetadata) {
> +  LLVMContext C;
> +  FunctionType *FTy =
> +      FunctionType::get(Type::getVoidTy(C), Type::getInt8Ty(C), false);
> +  std::unique_ptr<Function> F(
> +      Function::Create(FTy, GlobalValue::ExternalLinkage, "F"));
> +  Argument &A = *F->arg_begin();
> +
> +  auto *LAM = LocalAsMetadata::get(&A);
> +  auto *MAV = MetadataAsValue::get(C, LAM);
> +
> +  ValueToValueMapTy VM;
> +  EXPECT_EQ(nullptr, MapValue(MAV, VM));
> +  EXPECT_EQ(nullptr, MapValue(MAV, VM, RF_IgnoreMissingLocals));
> +  EXPECT_FALSE(VM.count(MAV));
> +  EXPECT_FALSE(VM.count(&A));
> +
> +  VM[MAV] = MAV;
> +  EXPECT_EQ(MAV, MapValue(MAV, VM));
> +  EXPECT_EQ(MAV, MapValue(MAV, VM, RF_IgnoreMissingLocals));
> +
> +  VM[MAV] = &A;
> +  EXPECT_EQ(&A, MapValue(MAV, VM));
> +  EXPECT_EQ(&A, MapValue(MAV, VM, RF_IgnoreMissingLocals));
> +}
> +
> } // end namespace
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list