From 2bf8abe1fef20f15d8e0971188bb5caf4b578c48 Mon Sep 17 00:00:00 2001 From: Nuno Cruces Date: Mon, 8 May 2023 18:04:02 +0100 Subject: [PATCH] Cleanup scattered documentation TODOs. (#1442) Signed-off-by: Nuno Cruces --- internal/engine/compiler/impl_amd64.go | 13 +++++++------ internal/platform/mmap_windows.go | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/internal/engine/compiler/impl_amd64.go b/internal/engine/compiler/impl_amd64.go index 4690e87d..9abdb9f0 100644 --- a/internal/engine/compiler/impl_amd64.go +++ b/internal/engine/compiler/impl_amd64.go @@ -1288,9 +1288,8 @@ func (c *amd64Compiler) compileClz(o *wazeroir.UnionOperation) error { c.assembler.CompileRegisterToRegister(amd64.LZCNTQ, target.register, target.register) } } else { - // On x86 mac, we cannot use LZCNT as it always results in zero. - // Instead we combine BSR (calculating most significant set bit) - // with XOR. This logic is described in + // On processors that do not support LZCNT, we combine BSR (calculating + // most significant set bit) with XOR. This logic is described in // "Replace Raw Assembly Code with Builtin Intrinsics" section in: // https://developer.apple.com/documentation/apple-silicon/addressing-architectural-differences-in-your-macos-code. @@ -1352,9 +1351,11 @@ func (c *amd64Compiler) compileCtz(o *wazeroir.UnionOperation) error { c.assembler.CompileRegisterToRegister(amd64.TZCNTQ, target.register, target.register) } } else { - // Somehow, if the target value is zero, TZCNT always returns zero: this is wrong. - // Meanwhile, we need branches for non-zero and zero cases on macos. - // TODO: find the reference to this behavior and put the link here. + // On processors that do not support TZCNT, the BSF instruction is + // executed instead. The key difference between TZCNT and BSF + // instruction is that if source operand is zero, the content of + // destination operand is undefined. + // https://www.felixcloutier.com/x86/tzcnt.html // First we compare the target with zero. c.assembler.CompileRegisterToConst(amd64.CMPQ, target.register, 0) diff --git a/internal/platform/mmap_windows.go b/internal/platform/mmap_windows.go index efa98a85..9c6fa622 100644 --- a/internal/platform/mmap_windows.go +++ b/internal/platform/mmap_windows.go @@ -29,7 +29,7 @@ func munmapCodeSegment(code []byte) error { // allocateMemory commits the memory region via the "VirtualAlloc" function. // See https://docs.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-virtualalloc func allocateMemory(size uintptr, protect uintptr) (uintptr, error) { - address := uintptr(0) // TODO: document why zero + address := uintptr(0) // system determines where to allocate the region. alloctype := windows_MEM_COMMIT if r, _, err := procVirtualAlloc.Call(address, size, alloctype, protect); r == 0 { return 0, fmt.Errorf("compiler: VirtualAlloc error: %w", ensureErr(err))