transform: improve GC stack slot pass to work around a bug

Bug 1790 ("musttail call must precede a ret with an optional bitcast")
is caused by the GC stack slot pass inserting a store instruction
between a musttail call and a return instruction. This is not allowed in
LLVM IR.

One solution would be to remove the musttail. That would probably work,
but 1) the go-llvm API doesn't support this and 2) this might have
unforeseen consequences. What I've done in this commit is to move the
store instruction to a position earlier in the basic block, just after
the last access to the GC stack slot alloca.

Thanks to @fgsch for a very small repro, which I've used as a regression
test.
This commit is contained in:
Ayke van Laethem
2021-07-31 19:17:07 +02:00
committed by Ron Evans
parent 98e70c9b19
commit ab47cea055
4 changed files with 61 additions and 3 deletions
+34 -2
View File
@@ -246,6 +246,7 @@ func MakeGCStackSlots(mod llvm.Module) bool {
}
// Do a store to the stack object after each new pointer that is created.
pointerStores := make(map[llvm.Value]struct{})
for i, ptr := range pointers {
// Insert the store after the pointer value is created.
insertionPoint := llvm.NextInstruction(ptr)
@@ -263,13 +264,44 @@ func MakeGCStackSlots(mod llvm.Module) bool {
}, "")
// Store the pointer into the stack slot.
builder.CreateStore(ptr, gep)
store := builder.CreateStore(ptr, gep)
pointerStores[store] = struct{}{}
}
// Make sure this stack object is popped from the linked list of stack
// objects at return.
for _, ret := range returns {
builder.SetInsertPointBefore(ret)
inst := ret
// Try to do the popping of the stack object earlier, by inserting
// it not right before the return instruction but moving the insert
// position up.
// This is necessary so that the GC stack slot pass doesn't
// interfere with tail calls (in particular, musttail calls).
for {
prevInst := llvm.PrevInstruction(inst)
if prevInst == parent {
break
}
if _, ok := pointerStores[prevInst]; ok {
// Pop the stack object after the last store instruction.
// This can probably be made more efficient: storing to the
// stack chain object and then immediately popping isn't
// useful.
break
}
if prevInst.IsNil() {
// Start of basic block. Pop the stack object here.
break
}
if !prevInst.IsAPHINode().IsNil() {
// Do not insert before a PHI node. PHI nodes must be
// grouped at the beginning of a basic block before any
// other instruction.
break
}
inst = prevInst
}
builder.SetInsertPointBefore(inst)
builder.CreateStore(parent, stackChainStart)
}
}