compress/lwz hangs building tests for target=wasi gc=conservative #2525
Comments
|
With |
dgryski
changed the title
|
How I usually debug these issues is by running it for a while and then killing it with SIGABRT. That will result in a panic-like message with stack trace. It seems to be stuck in |
|
Yeah, I found that last night. I wonder if something about the data structure we create is creating a loop in the EmitToMemoryBuffer code, although given that the memory size doesn't seem to be increasing at a huge rate, it's possible it's looping trying to look something up rather than looping trying to add something into the buffer. |
|
...but I can produce bitcode, which is useful for independent debugging: (The
That step is basically the whole machine code generation step, so a lot of stuff is happening there. |
|
Huh, that bitcode file is 4.1MB. That's pretty big. |
|
Trying bugpoint now. It's rather frustrating because I need to keep killing the process every time after 10 seconds or so. |
|
I'm trying to see if it happens with llvm 12, looking at the commit just before llvm13 support was merged into tinygo. |
|
Getting close. The problem is |
|
FYI: I'm running |
|
Actually, there's a much better command: |
|
Still happens with llvm12 at the checkout before llvm13 change landed. Building llvm11 and we'll see if it happens there too.. |
|
Still happens with llvm11 when the testenv stub was merged. Seems like this never worked, we just never got around to testing / noticing. |
|
Here is the buggy file: test.zip Extract it, and run |
|
Trying to minimize the lwz test source code, after a while I decided to try with Will keep plugging away at |
|
I think I might see what's going on here. Let me check... |
|
Ok, so I think what's going wrong here. The Reset function itself looks pretty harmless: func (r *Reader) Reset(src io.Reader, order Order, litWidth int) {
*r = Reader{}
r.init(src, order, litWidth)
}The issue however is that That would be roughly equivalent to the following Go code: func (r *Reader) Reset(src io.Reader, order Order, litWidth int):
var t0 Reader{}
t1 = *t0
*r = t1
r.init(src, order, litWidth) ()
returnSo it allocates a whole Clang does this a bit differently. For any zero store, it calls the LLVM memset function intrinsic which is later converted back to a regular store instruction or to an actual ...I can't think of a good fix right away. |
|
Can we call the memset intrinsic ourselves somehow? |
|
The problem is that this information is lost in Go SSA form, unless we recompute it. Which is a bit difficult. |
|
But yeah, once we know that a given store stores a zero value, we can trivially convert that to a memset. In fact, I already did that (but of course it didn't help): diff --git a/compiler/compiler.go b/compiler/compiler.go
index 6c0c98b2..14375e48 100644
--- a/compiler/compiler.go
+++ b/compiler/compiler.go
@@ -1192,6 +1199,14 @@ func (b *builder) createInstruction(instr ssa.Instruction) {
// nothing to store
return
}
+ if llvmVal == llvm.ConstNull(llvmVal.Type()) {
+ if llvmAddr.Type() != b.i8ptrType {
+ llvmAddr = b.CreateBitCast(llvmAddr, b.i8ptrType, "")
+ }
+ size := llvm.ConstInt(b.uintptrType, b.targetData.TypeAllocSize(llvmVal.Type()), false)
+ b.createMemset0Call(llvmAddr, size)
+ return
+ }
b.CreateStore(llvmVal, llvmAddr)
default:
b.addError(instr.Pos(), "unknown instruction: "+instr.String())
--- a/compiler/intrinsics.go
+++ b/compiler/intrinsics.go
@@ -33,6 +33,13 @@ func (b *builder) createMemoryCopyCall(fn *ssa.Function, args []ssa.Value) (llvm
// memory, declaring the function if needed. These calls will be lowered to
// regular libc memset calls if they aren't optimized out in a different way.
func (b *builder) createMemoryZeroCall(args []ssa.Value) (llvm.Value, error) {
+ ptr := b.getValue(args[0])
+ len := b.getValue(args[1])
+ b.createMemset0Call(ptr, len)
+ return llvm.Value{}, nil
+}
+
+func (b *builder) createMemset0Call(ptr, len llvm.Value) {
fnName := "llvm.memset.p0i8.i" + strconv.Itoa(b.uintptrType.IntTypeWidth())
llvmFn := b.mod.NamedFunction(fnName)
if llvmFn.IsNil() {
@@ -40,13 +47,12 @@ func (b *builder) createMemoryZeroCall(args []ssa.Value) (llvm.Value, error) {
llvmFn = llvm.AddFunction(b.mod, fnName, fnType)
}
params := []llvm.Value{
- b.getValue(args[0]),
+ ptr,
llvm.ConstInt(b.ctx.Int8Type(), 0, false),
- b.getValue(args[1]),
+ len,
llvm.ConstInt(b.ctx.Int1Type(), 0, false),
}
b.CreateCall(llvmFn, params, "")
- return llvm.Value{}, nil
}
var mathToLLVMMapping = map[string]string{ |
|
So this could hit any other large stack-allocated structure? |
|
Yes. |
|
So why doesn't this trigger with gc=leaking? |
|
The reason your patch didn't work was because the |
Yes. That's because it's not a constant (as I expected) but a load from that alloca ( |
|
Last night I was playing with trying to detect "if the previous use of this ssa value was in an alloc" , but rapidly ran into my knowledge of both llvm and Go's SSA package. Just an idea, anyway. |
|
(Just to update: This is still happening) |
|
This is now fixed (but not sure why/how). |
|
#3628 <-- compress/lzw now runs tests under wasi. |
|
This is part of the |
Just realized I had this running for over a hour trying to build. This isn't the interp phase (which would time out) nor the test itself (since we would see the wasmtime command line.). Just building. Maybe an infinite-ish loop somewhere?
The text was updated successfully, but these errors were encountered: