[llvm] r264320 - [Statepoints] Fix yet another issue around gc pointer uniqueing
Sanjoy Das via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 24 11:57:39 PDT 2016
Author: sanjoy
Date: Thu Mar 24 13:57:39 2016
New Revision: 264320
URL: http://llvm.org/viewvc/llvm-project?rev=264320&view=rev
Log:
[Statepoints] Fix yet another issue around gc pointer uniqueing
Given that StatepointLowering now uniques derived pointers before
putting them in the per-statepoint spill map, we may end up with missing
entries for derived pointers when we visit a gc.relocate on a pointer
that was de-duplicated away.
Fix this by keeping two maps, one mapping gc pointers to their
de-duplicated values, and one mapping a de-duplicated value to the slot
it is spilled in.
Added:
llvm/trunk/test/CodeGen/X86/statepoint-uniqueing.ll
- copied, changed from r264319, llvm/trunk/test/CodeGen/X86/statepoint-duplicate-gcrelocate.ll
Removed:
llvm/trunk/test/CodeGen/X86/statepoint-duplicate-gcrelocate.ll
Modified:
llvm/trunk/include/llvm/CodeGen/FunctionLoweringInfo.h
llvm/trunk/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
Modified: llvm/trunk/include/llvm/CodeGen/FunctionLoweringInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/FunctionLoweringInfo.h?rev=264320&r1=264319&r2=264320&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/FunctionLoweringInfo.h (original)
+++ llvm/trunk/include/llvm/CodeGen/FunctionLoweringInfo.h Thu Mar 24 13:57:39 2016
@@ -81,13 +81,35 @@ public:
DenseMap<const Value *, unsigned> CatchPadExceptionPointers;
/// Keep track of frame indices allocated for statepoints as they could be
- /// used across basic block boundaries. Key of the map is statepoint
- /// instruction, value is a map from spilled llvm Value to the optional stack
- /// stack slot index. If optional is unspecified it means that we have
- /// visited this value but didn't spill it.
- typedef DenseMap<const Value*, Optional<int>> StatepointSpilledValueMapTy;
- DenseMap<const Instruction*, StatepointSpilledValueMapTy>
- StatepointRelocatedValues;
+ /// used across basic block boundaries. This struct is more complex than a
+ /// simple map because the stateopint lowering code de-duplicates gc pointers
+ /// based on their SDValue (so %p and (bitcast %p to T) will get the same
+ /// slot), and we track that here.
+
+ struct StatepointSpillMap {
+ typedef DenseMap<const Value *, Optional<int>> SlotMapTy;
+
+ /// Maps uniqued llvm IR values to the slots they were spilled in. If a
+ /// value is mapped to None it means we visited the value but didn't spill
+ /// it (because it was a constant, for instance).
+ SlotMapTy SlotMap;
+
+ /// Maps llvm IR values to the values they were de-duplicated to.
+ DenseMap<const Value *, const Value *> DuplicateMap;
+
+ SlotMapTy::const_iterator find(const Value *V) const {
+ auto DuplIt = DuplicateMap.find(V);
+ if (DuplIt != DuplicateMap.end())
+ V = DuplIt->second;
+ return SlotMap.find(V);
+ }
+
+ SlotMapTy::const_iterator end() const { return SlotMap.end(); }
+ };
+
+ /// Maps gc.statepoint instructions to their corresponding StatepointSpillMap
+ /// instances.
+ DenseMap<const Instruction *, StatepointSpillMap> StatepointSpillMaps;
/// StaticAllocaMap - Keep track of frame indices for fixed sized allocas in
/// the entry block. This allows the allocas to be efficiently referenced
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp?rev=264320&r1=264319&r2=264320&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp Thu Mar 24 13:57:39 2016
@@ -356,7 +356,7 @@ void FunctionLoweringInfo::clear() {
ByValArgFrameIndexMap.clear();
RegFixups.clear();
StatepointStackSlots.clear();
- StatepointRelocatedValues.clear();
+ StatepointSpillMaps.clear();
PreferredExtendType.clear();
}
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp?rev=264320&r1=264319&r2=264320&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp Thu Mar 24 13:57:39 2016
@@ -124,7 +124,7 @@ static Optional<int> findPreviousSpillSl
// Spill location is known for gc relocates
if (const auto *Relocate = dyn_cast<GCRelocateInst>(Val)) {
const auto &SpillMap =
- Builder.FuncInfo.StatepointRelocatedValues[Relocate->getStatepoint()];
+ Builder.FuncInfo.StatepointSpillMaps[Relocate->getStatepoint()];
auto It = SpillMap.find(Relocate->getDerivedPtr());
if (It == SpillMap.end())
@@ -249,22 +249,26 @@ static void
removeDuplicateGCPtrs(SmallVectorImpl<const Value *> &Bases,
SmallVectorImpl<const Value *> &Ptrs,
SmallVectorImpl<const GCRelocateInst *> &Relocs,
- SelectionDAGBuilder &Builder) {
-
- // This is horribly inefficient, but I don't care right now
- SmallSet<SDValue, 32> Seen;
+ SelectionDAGBuilder &Builder,
+ FunctionLoweringInfo::StatepointSpillMap &SSM) {
+ DenseMap<SDValue, const Value *> Seen;
SmallVector<const Value *, 64> NewBases, NewPtrs;
SmallVector<const GCRelocateInst *, 64> NewRelocs;
for (size_t i = 0, e = Ptrs.size(); i < e; i++) {
SDValue SD = Builder.getValue(Ptrs[i]);
- // Only add non-duplicates
- if (Seen.count(SD) == 0) {
+ auto SeenIt = Seen.find(SD);
+
+ if (SeenIt == Seen.end()) {
+ // Only add non-duplicates
NewBases.push_back(Bases[i]);
NewPtrs.push_back(Ptrs[i]);
NewRelocs.push_back(Relocs[i]);
+ Seen[SD] = Ptrs[i];
+ } else {
+ // Duplicate pointer found, note in SSM and move on:
+ SSM.DuplicateMap[Ptrs[i]] = SeenIt->second;
}
- Seen.insert(SD);
}
assert(Bases.size() >= NewBases.size());
assert(Ptrs.size() >= NewPtrs.size());
@@ -499,7 +503,7 @@ lowerStatepointMetaArgs(SmallVectorImpl<
// This can not be embedded in lowering loops as we need to record *all*
// values, while previous loops account only values with unique SDValues.
const Instruction *StatepointInstr = SI.StatepointInstr;
- auto &SpillMap = Builder.FuncInfo.StatepointRelocatedValues[StatepointInstr];
+ auto &SpillMap = Builder.FuncInfo.StatepointSpillMaps[StatepointInstr];
for (const GCRelocateInst *Relocate : SI.GCRelocates) {
const Value *V = Relocate->getDerivedPtr();
@@ -507,7 +511,7 @@ lowerStatepointMetaArgs(SmallVectorImpl<
SDValue Loc = Builder.StatepointLowering.getLocation(SDV);
if (Loc.getNode()) {
- SpillMap[V] = cast<FrameIndexSDNode>(Loc)->getIndex();
+ SpillMap.SlotMap[V] = cast<FrameIndexSDNode>(Loc)->getIndex();
} else {
// Record value as visited, but not spilled. This is case for allocas
// and constants. For this values we can avoid emitting spill load while
@@ -515,7 +519,7 @@ lowerStatepointMetaArgs(SmallVectorImpl<
// Actually we do not need to record them in this map at all.
// We do this only to check that we are not relocating any unvisited
// value.
- SpillMap[V] = None;
+ SpillMap.SlotMap[V] = None;
// Default llvm mechanisms for exporting values which are used in
// different basic blocks does not work for gc relocates.
@@ -551,7 +555,8 @@ SDValue SelectionDAGBuilder::LowerAsSTAT
// input. Also has the effect of removing duplicates in the original
// llvm::Value input list as well. This is a useful optimization for
// reducing the size of the StackMap section. It has no other impact.
- removeDuplicateGCPtrs(SI.Bases, SI.Ptrs, SI.GCRelocates, *this);
+ removeDuplicateGCPtrs(SI.Bases, SI.Ptrs, SI.GCRelocates, *this,
+ FuncInfo.StatepointSpillMaps[SI.StatepointInstr]);
assert(SI.Bases.size() == SI.Ptrs.size() &&
SI.Ptrs.size() == SI.GCRelocates.size());
@@ -886,12 +891,10 @@ void SelectionDAGBuilder::visitGCRelocat
const Value *DerivedPtr = Relocate.getDerivedPtr();
SDValue SD = getValue(DerivedPtr);
- FunctionLoweringInfo::StatepointSpilledValueMapTy &SpillMap =
- FuncInfo.StatepointRelocatedValues[Relocate.getStatepoint()];
-
- // We should have recorded location for this pointer
- assert(SpillMap.count(DerivedPtr) && "Relocating not lowered gc value");
- Optional<int> DerivedPtrLocation = SpillMap[DerivedPtr];
+ auto &SpillMap = FuncInfo.StatepointSpillMaps[Relocate.getStatepoint()];
+ auto SlotIt = SpillMap.find(DerivedPtr);
+ assert(SlotIt != SpillMap.end() && "Relocating not lowered gc value");
+ Optional<int> DerivedPtrLocation = SlotIt->second;
// We didn't need to spill these special cases (constants and allocas).
// See the handling in spillIncomingValueForStatepoint for detail.
Removed: llvm/trunk/test/CodeGen/X86/statepoint-duplicate-gcrelocate.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/statepoint-duplicate-gcrelocate.ll?rev=264319&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/X86/statepoint-duplicate-gcrelocate.ll (original)
+++ llvm/trunk/test/CodeGen/X86/statepoint-duplicate-gcrelocate.ll (removed)
@@ -1,20 +0,0 @@
-; RUN: llc < %s | FileCheck %s
-
-; Checks for a crash we had when two gc.relocate calls would
-; relocating identical values
-
-target datalayout = "e-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-pc-linux-gnu"
-
-declare void @f()
-declare token @llvm.experimental.gc.statepoint.p0f_isVoidf(i64, i32, void ()*, i32, i32, ...)
-declare i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token, i32, i32) #3
-
-define void @test(i32 addrspace(1)* %ptr) gc "statepoint-example" {
-; CHECK-LABEL: test
- %tok = tail call token (i64, i32, void ()*, i32, i32, ...)
- @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @f, i32 0, i32 0, i32 0, i32 2, i32 addrspace(1)* %ptr, i32 undef, i32 addrspace(1)* %ptr, i32 addrspace(1)* %ptr)
- %a = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token %tok, i32 9, i32 9)
- %b = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token %tok, i32 10, i32 10)
- ret void
-}
Copied: llvm/trunk/test/CodeGen/X86/statepoint-uniqueing.ll (from r264319, llvm/trunk/test/CodeGen/X86/statepoint-duplicate-gcrelocate.ll)
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/statepoint-uniqueing.ll?p2=llvm/trunk/test/CodeGen/X86/statepoint-uniqueing.ll&p1=llvm/trunk/test/CodeGen/X86/statepoint-duplicate-gcrelocate.ll&r1=264319&r2=264320&rev=264320&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/statepoint-duplicate-gcrelocate.ll (original)
+++ llvm/trunk/test/CodeGen/X86/statepoint-uniqueing.ll Thu Mar 24 13:57:39 2016
@@ -9,12 +9,23 @@ target triple = "x86_64-pc-linux-gnu"
declare void @f()
declare token @llvm.experimental.gc.statepoint.p0f_isVoidf(i64, i32, void ()*, i32, i32, ...)
declare i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token, i32, i32) #3
+declare i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token, i32, i32) #3
-define void @test(i32 addrspace(1)* %ptr) gc "statepoint-example" {
-; CHECK-LABEL: test
+define void @test_gcrelocate_uniqueing(i32 addrspace(1)* %ptr) gc "statepoint-example" {
+; CHECK-LABEL: test_gcrelocate_uniqueing
%tok = tail call token (i64, i32, void ()*, i32, i32, ...)
@llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @f, i32 0, i32 0, i32 0, i32 2, i32 addrspace(1)* %ptr, i32 undef, i32 addrspace(1)* %ptr, i32 addrspace(1)* %ptr)
%a = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token %tok, i32 9, i32 9)
%b = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token %tok, i32 10, i32 10)
ret void
}
+
+define void @test_gcptr_uniqueing(i32 addrspace(1)* %ptr) gc "statepoint-example" {
+; CHECK-LABEL: test_gcptr_uniqueing
+ %ptr2 = bitcast i32 addrspace(1)* %ptr to i8 addrspace(1)*
+ %tok = tail call token (i64, i32, void ()*, i32, i32, ...)
+ @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @f, i32 0, i32 0, i32 0, i32 2, i32 addrspace(1)* %ptr, i32 undef, i32 addrspace(1)* %ptr, i8 addrspace(1)* %ptr2)
+ %a = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token %tok, i32 9, i32 9)
+ %b = call i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %tok, i32 10, i32 10)
+ ret void
+}
More information about the llvm-commits
mailing list