RPCS3/llvm
1
0
Fork 0
mirror of https://github.com/RPCS3/llvm.git synced 2024-12-14 07:31:53 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Revert "ValueMapper: Treat LocalAsMetadata more like function-local Values"

Browse Source 
This reverts commit r265759, since even this limited version breaks some
bots:
  http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/3311
  http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/17696

This also reverts r265761 "ValueMapper: Unduplicate
RF_NoModuleLevelChanges check, NFC", since I had trouble separating it
from r265759.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@265765 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Duncan P. N. Exon Smith 2016-04-08 00:56:21 +00:00
parent 6831b38fa2
commit 50c7f32d75
5 changed files with 30 additions and 243 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
include/llvm/Transforms/Utils/ValueMapper.h
View File

@ -68,21 +68,13 @@ enum RemapFlags {
RF_NoModuleLevelChanges = 1,
/// 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.
/// (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.
///
/// There are no such assertions in MapValue(), whose results are almost
/// unchanged by this flag. This flag mainly changes the assertion behaviour
/// in RemapInstruction().
///
/// Since an Instruction's metadata operands (even that point to SSA values)
/// aren't guaranteed to be dominated by their definitions, MapMetadata will
/// return "!{}" instead of "null" for \a LocalAsMetadata instances whose SSA
/// values are unmapped when this flag is set. Otherwise, \a MapValue()
/// completely ignores this flag.
///
/// \a MapMetadata() always ignores this flag.
/// 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
@ -109,32 +101,12 @@ static inline RemapFlags operator|(RemapFlags LHS, RemapFlags RHS) {
/// 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 if \c V is a \a MetadataAsValue wrapping a LocalAsMetadata,
/// recurse on the local SSA value, and return nullptr or "metadata !{}" on
/// missing depending on RF_IgnoreMissingValues.
/// 6. Else if \c V is a \a MetadataAsValue, rewrap the return of \a
/// MapMetadata().
/// 7. Else, compute the equivalent constant, and return it.
/// 5. Else, Compute the equivalent constant, and return it.
Value *MapValue(const Value *V, ValueToValueMapTy &VM,
RemapFlags Flags = RF_None,
ValueMapTypeRemapper *TypeMapper = nullptr,
ValueMaterializer *Materializer = nullptr);
/// Lookup or compute a mapping for a piece of metadata.
///
/// Compute and memoize a mapping for \c MD.
///
/// 1. If \c MD is mapped, return it.
/// 2. Else if \a RF_NoModuleLevelChanges or \c MD is an \a MDString, return
/// \c MD.
/// 3. Else if \c MD is a \a ConstantAsMetadata, call \a MapValue() and
/// re-wrap its return (returning nullptr on nullptr).
/// 4. Else, \c MD is an \a MDNode. These are remapped, along with their
/// transitive operands. Distinct nodes are duplicated or moved depending
/// on \a RF_MoveDistinctNodes. Uniqued nodes are remapped like constants.
///
/// \note \a LocalAsMetadata is completely unsupported by \a MapMetadata.
/// Instead, use \a MapValue() with its wrapping \a MetadataAsValue instance.
Metadata *MapMetadata(const Metadata *MD, ValueToValueMapTy &VM,
RemapFlags Flags = RF_None,
ValueMapTypeRemapper *TypeMapper = nullptr,

4
lib/IR/Verifier.cpp
View File

@ -3502,10 +3502,6 @@ void Verifier::verifyDominatesUse(Instruction &I, unsigned i) {
// Quick check whether the def has already been encountered in the same block.
// PHI nodes are not checked to prevent accepting preceeding PHIs, because PHI
// uses are defined to happen on the incoming edge, not at the instruction.
//
// FIXME: If this operand is a MetadataAsValue (wrapping a LocalAsMetadata)
// wrapping an SSA value, assert that we've already encountered it. See
// related FIXME in Mapper::mapLocalAsMetadata in ValueMapper.cpp.
if (!isa<PHINode>(I) && InstsInThisBlock.count(Op))
return;

83
lib/Transforms/Utils/ValueMapper.cpp
View File

@ -86,20 +86,6 @@ public:
/// (not an MDNode, or MDNode::isResolved() returns true).
Metadata *mapMetadata(const Metadata *MD);
// Map LocalAsMetadata, which never gets memoized.
//
// If the referenced local is not mapped, the principled return is nullptr.
// However, optimization passes sometimes move metadata operands *before* the
// SSA values they reference. To prevent crashes in \a RemapInstruction(),
// return "!{}" when RF_IgnoreMissingLocals is not set.
//
// \note Adding a mapping for LocalAsMetadata is unsupported. Add a mapping
// to the value map for the SSA value in question instead.
//
// FIXME: Once we have a verifier check for forward references to SSA values
// through metadata operands, always return nullptr on unmapped locals.
Metadata *mapLocalAsMetadata(const LocalAsMetadata &LAM);
private:
Value *mapBlockAddress(const BlockAddress &BA);
@ -308,32 +294,18 @@ Value *Mapper::mapValue(const Value *V) {
if (const auto *MDV = dyn_cast<MetadataAsValue>(V)) {
const Metadata *MD = MDV->getMetadata();
if (auto *LAM = dyn_cast<LocalAsMetadata>(MD)) {
// Look through to grab the local value.
if (Value *LV = mapValue(LAM->getValue())) {
if (V == LAM->getValue())
return const_cast<Value *>(V);
return MetadataAsValue::get(V->getContext(), LocalAsMetadata::get(LV));
}
// FIXME: always return nullptr once Verifier::verifyDominatesUse()
// ensures metadata operands only reference defined SSA values.
return (Flags & RF_IgnoreMissingLocals)
? nullptr
: MetadataAsValue::get(V->getContext(),
MDTuple::get(V->getContext(), None));
}
// If this is a module-level metadata and we know that nothing at the module
// level is changing, then use an identity mapping.
if (Flags & RF_NoModuleLevelChanges)
if (!isa<LocalAsMetadata>(MD) && (Flags & RF_NoModuleLevelChanges))
return VM[V] = const_cast<Value *>(V);
// Map the metadata and turn it into a value.
// 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)
if (MD == MappedMD || (!MappedMD && (Flags & RF_IgnoreMissingLocals)))
return VM[V] = const_cast<Value *>(V);
return VM[V] = MetadataAsValue::get(V->getContext(), MappedMD);
}
@ -651,18 +623,21 @@ Optional<Metadata *> Mapper::mapSimpleMetadata(const Metadata *MD) {
if (isa<MDString>(MD))
return mapToSelf(MD);
// This is a module-level metadata. If nothing at the module level is
// changing, use an identity mapping.
if ((Flags & RF_NoModuleLevelChanges))
return mapToSelf(MD);
if (isa<ConstantAsMetadata>(MD))
if ((Flags & RF_NoModuleLevelChanges))
return mapToSelf(MD);
if (auto *CMD = dyn_cast<ConstantAsMetadata>(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(CMD->getValue());
Value *MappedV = mapValue(VMD->getValue());
VM.enableMapMetadata();
if (CMD->getValue() == MappedV)
// FIXME: Always use "ignore" behaviour. There should only be globals here.
if (VMD->getValue() == MappedV ||
(!MappedV && (Flags & RF_IgnoreMissingLocals)))
return mapToSelf(MD);
return mapToMetadata(MD, MappedV ? ValueAsMetadata::get(MappedV) : nullptr);
@ -670,6 +645,11 @@ Optional<Metadata *> Mapper::mapSimpleMetadata(const Metadata *MD) {
assert(isa<MDNode>(MD) && "Expected a metadata node");
// If this is a module-level metadata and we know that nothing at the
// module level is changing, then use an identity mapping.
if (Flags & RF_NoModuleLevelChanges)
return mapToSelf(MD);
return None;
}
@ -679,26 +659,9 @@ Metadata *llvm::MapMetadata(const Metadata *MD, ValueToValueMapTy &VM,
return Mapper(VM, Flags, TypeMapper, Materializer).mapMetadata(MD);
}
Metadata *Mapper::mapLocalAsMetadata(const LocalAsMetadata &LAM) {
// 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);
}
// FIXME: always return nullptr once Verifier::verifyDominatesUse() ensures
// metadata operands only reference defined SSA values.
return (Flags & RF_IgnoreMissingLocals)
? nullptr
: MDTuple::get(LAM.getContext(), None);
}
Metadata *Mapper::mapMetadata(const Metadata *MD) {
assert(MD && "Expected valid metadata");
assert(!isa<LocalAsMetadata>(MD) && "Unexpected local metadata");
// 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;

49
test/Transforms/Inline/local-as-metadata-undominated-use.ll
View File

@ -1,49 +0,0 @@
; RUN: opt -inline -S < %s | FileCheck %s
; Make sure the inliner doesn't crash when a metadata-bridged SSA operand is an
; undominated use.
;
; If we ever add a verifier check to prevent the scenario in this file, it's
; fine to delete this testcase. However, we would need a bitcode upgrade since
; such historical IR exists in practice.
define i32 @foo(i32 %i) !dbg !4 {
entry:
tail call void @llvm.dbg.value(metadata i32 %add, i64 0, metadata !8, metadata !10), !dbg !11
%add = add nsw i32 1, %i, !dbg !12
ret i32 %add, !dbg !13
}
; CHECK-LABEL: define i32 @caller(
define i32 @caller(i32 %i) {
; CHECK-NEXT: entry:
entry:
; Although the inliner shouldn't crash, it can't be expected to get the
; "correct" SSA value since its assumptions have been violated.
; CHECK-NEXT: tail call void @llvm.dbg.value(metadata ![[EMPTY:[0-9]+]],
; CHECK-NEXT: %{{.*}} = add nsw
%call = tail call i32 @foo(i32 %i)
ret i32 %call
}
declare void @llvm.dbg.value(metadata, i64, metadata, metadata)
!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!9}
!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.9.0 (trunk 265634) (llvm/trunk 265637)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, subprograms: !3)
!1 = !DIFile(filename: "t.c", directory: "/path/to/tests")
; CHECK: ![[EMPTY]] = !{}
!2 = !{}
!3 = !{!4}
!4 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 2, type: !5, isLocal: false, isDefinition: true, scopeLine: 2, flags: DIFlagPrototyped, isOptimized: true)
!5 = !DISubroutineType(types: !6)
!6 = !{!7, !7}
!7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
!8 = !DILocalVariable(name: "add", arg: 1, scope: !4, file: !1, line: 2, type: !7)
!9 = !{i32 2, !"Debug Info Version", i32 3}
!10 = !DIExpression()
!11 = !DILocation(line: 2, column: 13, scope: !4)
!12 = !DILocation(line: 2, column: 27, scope: !4)
!13 = !DILocation(line: 2, column: 18, scope: !4)

95
unittests/Transforms/Utils/ValueMapperTest.cpp
View File

@ -139,99 +139,4 @@ TEST(ValueMapperTest, MapMetadataNullMapGlobalWithIgnoreMissingLocals) {
EXPECT_EQ(nullptr, MapValue(F.get(), VM, Flags));
}
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 = MDTuple::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());
}
#ifdef GTEST_HAS_DEATH_TEST
#ifndef NDEBUG
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();
// MapMetadata doesn't support LocalAsMetadata. The only valid container for
// LocalAsMetadata is a MetadataAsValue instance, so use it directly.
auto *LAM = LocalAsMetadata::get(&A);
ValueToValueMapTy VM;
EXPECT_DEATH(MapMetadata(LAM, VM), "Unexpected local metadata");
EXPECT_DEATH(MapMetadata(LAM, VM, RF_IgnoreMissingLocals),
"Unexpected local metadata");
}
#endif
#endif
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);
// The principled answer to a LocalAsMetadata of an unmapped SSA value would
// be to return nullptr (regardless of RF_IgnoreMissingLocals).
//
// However, algorithms that use RemapInstruction assume that each instruction
// only references SSA values from previous instructions. Arguments of
// such as "metadata i32 %x" don't currently successfully maintain that
// property. To keep RemapInstruction from crashing we need a non-null
// return here, but we also shouldn't reference the unmapped local. Use
// "metadata !{}".
auto *N0 = MDTuple::get(C, None);
auto *N0AV = MetadataAsValue::get(C, N0);
ValueToValueMapTy VM;
EXPECT_EQ(N0AV, MapValue(MAV, VM));
EXPECT_EQ(nullptr, MapValue(MAV, VM, RF_IgnoreMissingLocals));
EXPECT_FALSE(VM.count(MAV));
EXPECT_FALSE(VM.count(&A));
EXPECT_EQ(None, VM.getMappedMD(LAM));
VM[MAV] = MAV;
EXPECT_EQ(MAV, MapValue(MAV, VM));
EXPECT_EQ(MAV, MapValue(MAV, VM, RF_IgnoreMissingLocals));
EXPECT_TRUE(VM.count(MAV));
EXPECT_FALSE(VM.count(&A));
VM[MAV] = &A;
EXPECT_EQ(&A, MapValue(MAV, VM));
EXPECT_EQ(&A, MapValue(MAV, VM, RF_IgnoreMissingLocals));
EXPECT_TRUE(VM.count(MAV));
EXPECT_FALSE(VM.count(&A));
}
} // end namespace