Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vector of bool trips LLVM assertion "Invalid vector size" #11587

Closed
gwenzek opened this issue May 5, 2022 · 4 comments
Closed

Vector of bool trips LLVM assertion "Invalid vector size" #11587

gwenzek opened this issue May 5, 2022 · 4 comments
Labels
backend-llvm The LLVM backend outputs an LLVM IR Module. bug Observed behavior contradicts documented or intended behavior stage1 The process of building from source via WebAssembly and the C backend.
Milestone

Comments

@gwenzek
Copy link
Contributor

gwenzek commented May 5, 2022

Zig Version

0.9.0+my_fork https://github.com/gwenzek/zig/tree/llvm14
(To work on #11274 )

Steps to Reproduce

Simplified version of test/behavior/vector.zig

const std = @import("std");
const Vector = std.meta.Vector;

export fn vector_bitcast() u8 {
    const a: Vector(4, bool) = [_]bool{ true, false, true, false };
    const result_array: [4]bool = a;
    return if (result_array[0]) 0 else 1;
}
$ /home/guw/github/zig-bootstrap/out/build-zig-host/zig build-obj -I test --zig-lib-dir lib -Denable-llvm -Dconfig_h=/home/guw/github/zig-bootstrap/out/build-zig-host/config.h  test/behavior/vector.zig
LLVM Emit Object... zig: /home/guw/github/zig-bootstrap/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1475: bool hasVectorBeenPadded(const llvm::DICompositeType*): Assertion `ActualSize >= (NumVecElements * ElementSize) && "Invalid vector size"' failed.

Note: I'm using LLVM14 built with assertions on, but I don't think that LLVM14 is the main issue since this assertion is 4 years old
https://github.com/llvm/llvm-project/blame/75f9e83ace52773af65dcebca543005ec8a2705d/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp#L1473

A reduced version of the LLVM IR is:

define i8 @vector_bitcast() !dbg !5 {
Entry:
  store i8 0, i8* null, align 1, !dbg !19
  ret i8 0
}

!llvm.module.flags = !{!0, !1}
!llvm.dbg.cu = !{!2}

!0 = !{i32 2, !"Debug Info Version", i32 3}
!1 = !{i32 2, !"Dwarf Version", i32 4}
!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "zig 0.9.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !4)
!3 = !DIFile(filename: "vector", directory: "test/behavior")
!4 = !{}
!5 = distinct !DISubprogram(name: "vector_bitcast", scope: !6, file: !6, line: 22, type: !7, scopeLine: 22, flags: DIFlagStaticMember, spFlags: DISPFlagDefinition, unit: !2, retainedNodes: !10)
!6 = !DIFile(filename: "vector.zig", directory: "/home/guw/github/zig-bootstrap/zig/test/behavior")
!7 = !DISubroutineType(types: !8)
!8 = !{!9}
!9 = !DIBasicType(name: "u8", size: 8, encoding: DW_ATE_unsigned_char)
!10 = !{!11, !17}
!11 = !DILocalVariable(name: "a", scope: !12, file: !6, line: 23, type: !13)
!12 = distinct !DILexicalBlock(scope: !5, file: !6, line: 22, column: 31)
!13 = !DICompositeType(tag: DW_TAG_array_type, baseType: !14, size: 8, align: 1, flags: DIFlagVector, elements: !15)
!14 = !DIBasicType(name: "bool", size: 8, encoding: DW_ATE_boolean)
!15 = !{!16}
!16 = !DISubrange(count: 4, lowerBound: 0)
!17 = !DILocalVariable(name: "result_array", scope: !12, file: !6, line: 24, type: !18)
!18 = !DICompositeType(tag: DW_TAG_array_type, baseType: !14, size: 32, align: 32, elements: !15)
!19 = !DILocation(line: 25, column: 33, scope: !12)

Calling llc on it will also fail with the same error (unless built without assertions):

[guw@jinx-fedora zig]$ /home/guw/github/zig-bootstrap/out/host/bin/llc vector.ll 
llc: /home/guw/github/zig-bootstrap/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1475: bool hasVectorBeenPadded(const llvm::DICompositeType*): Assertion `ActualSize >= (NumVecElements * ElementSize) && "Invalid vector size"' failed.

As you can see from the IR, the bug is only due to the debug info and not to the actual code.
The issue is that:

  • a is a vector of 4 booleans
  • a has a size of 8 bits
  • booleans have a size of 8 bits too
  • DwarfUnit.cpp doesn't understand how we can fit 4 8-bits booleans inside an 8 bits vector.

Expected Behavior

I'm not sure what's the correct way of fixing this, but we shouldn't have an assertion error while writing the Dwarf Debug Info

Either:

  • Zig should say that the vector is made of 1-bit booleans
  • DwarfUnit.cpp shouldn't try to assert the vector size (I guess the reader of debug info should make this kind of checks if it's relevant to them)

Actual Behavior

/home/guw/github/zig-bootstrap/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1475: bool hasVectorBeenPadded(const llvm::DICompositeType*): Assertion `ActualSize >= (NumVecElements * ElementSize) && "Invalid vector size"' failed.
@gwenzek gwenzek added the bug Observed behavior contradicts documented or intended behavior label May 5, 2022
@Vexu Vexu added the backend-llvm The LLVM backend outputs an LLVM IR Module. label Jun 1, 2022
@Vexu Vexu added this to the 0.11.0 milestone Jun 1, 2022
@andrewrk andrewrk modified the milestones: 0.11.0, 0.10.0 Jul 4, 2022
@andrewrk andrewrk added the stage1 The process of building from source via WebAssembly and the C backend. label Jul 4, 2022
@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Jul 4, 2022
@andrewrk
Copy link
Member

andrewrk commented Jul 4, 2022

I confirmed this affects stage1 but not stage2.

@andrewrk andrewrk added the upstream An issue with a third party project that Zig uses. label Jul 4, 2022
@andrewrk
Copy link
Member

andrewrk commented Jul 4, 2022

Given that this works fine if you change it to a vector of u8:

export fn vector_bitcast() u8 {
    const a: @Vector(4, u8) = [_]u8{ 1, 2, 3, 4 };
    const result_array: [4]u8 = a;
    return result_array[0];
}

I think this is actually llvm/llvm-project#55522.

I also tried this patch which had no effect:

--- a/src/codegen/llvm.zig
+++ b/src/codegen/llvm.zig
@@ -1476,10 +1476,17 @@ pub const Object = struct {
                 return array_di_ty;
             },
             .Vector => {
+                // Lower vector of bool with the element type being a u1 instead so that
+                // LLVM debug info does not get confused and think that the element type has
+                // 8 bits instead of 1.
+                const elem_di_ty = if (ty.childType().zigTypeTag() == .Bool)
+                    try o.lowerDebugType(Type.u1, .full)
+                else
+                    try o.lowerDebugType(ty.childType(), .full);
                 const vector_di_ty = dib.createVectorType(
                     ty.abiSize(target) * 8,
                     ty.abiAlignment(target) * 8,
-                    try o.lowerDebugType(ty.childType(), .full),
+                    elem_di_ty,
                     ty.vectorLen(),
                 );
                 // The recursive call to `lowerDebugType` means we can't use `gop` anymore.
--- a/src/stage1/analyze.cpp
+++ b/src/stage1/analyze.cpp
@@ -9933,8 +9933,14 @@ static void resolve_llvm_types(CodeGen *g, ZigType *type, ResolveStatus wanted_r
             if (type->llvm_di_type != nullptr) return;
 
             type->llvm_type = LLVMVectorType(get_llvm_type(g, type->data.vector.elem_type), type->data.vector.len);
-            type->llvm_di_type = ZigLLVMDIBuilderCreateVectorType(g->dbuilder, 8 * type->abi_size,
-                    type->abi_align, get_llvm_di_type(g, type->data.vector.elem_type), type->data.vector.len);
+            ZigLLVMDIType *elem_di_ty = type->data.vector.elem_type->id == ZigTypeIdBool ?
+                get_llvm_di_type(g, get_int_type(g, false, 1)) :
+                get_llvm_di_type(g, type->data.vector.elem_type) ;
+            type->llvm_di_type = ZigLLVMDIBuilderCreateVectorType(g->dbuilder,
+                    8 * type->abi_size,
+                    8 * type->abi_align,
+                    elem_di_ty,
+                    type->data.vector.len);
             return;
         }
         case ZigTypeIdFnFrame:

We don't actually have a zig issue specifically open to track that upstream issue, however, so this one can be it.

@andrewrk andrewrk changed the title "Wrong" Debug Information generated for Vector of bool Vector of bool trips LLVM assertion "Invalid vector size" Jul 4, 2022
@andrewrk
Copy link
Member

andrewrk commented Jul 4, 2022

Hmm looks like actually this is a separate issue from that LLVM issue because this diff did solve the issue:

--- a/src/stage1/analyze.cpp
+++ b/src/stage1/analyze.cpp
@@ -9933,8 +9933,15 @@ static void resolve_llvm_types(CodeGen *g, ZigType *type, ResolveStatus wanted_r
             if (type->llvm_di_type != nullptr) return;
 
             type->llvm_type = LLVMVectorType(get_llvm_type(g, type->data.vector.elem_type), type->data.vector.len);
-            type->llvm_di_type = ZigLLVMDIBuilderCreateVectorType(g->dbuilder, 8 * type->abi_size,
-                    type->abi_align, get_llvm_di_type(g, type->data.vector.elem_type), type->data.vector.len);
+            ZigLLVMDIType *elem_di_ty = type->data.vector.elem_type->id == ZigTypeIdBool ?
+                ZigLLVMCreateDebugBasicType(g->dbuilder, "bool", 1,
+                        ZigLLVMEncoding_DW_ATE_unsigned()) :
+                get_llvm_di_type(g, type->data.vector.elem_type) ;
+            type->llvm_di_type = ZigLLVMDIBuilderCreateVectorType(g->dbuilder,
+                    8 * type->abi_size,
+                    8 * type->abi_align,
+                    elem_di_ty,
+                    type->data.vector.len);
             return;
         }
         case ZigTypeIdFnFrame:

stage2 does not have this issue because integer and boolean types use the proper bit size rather than abi_size * 8.

@andrewrk andrewrk removed the upstream An issue with a third party project that Zig uses. label Jul 4, 2022
@andrewrk andrewrk modified the milestones: 0.11.0, 0.10.0 Jul 4, 2022
@gwenzek
Copy link
Contributor Author

gwenzek commented Jul 4, 2022

@andrewrk here is how I was planning to solve this issue: in codegen.cpp@add_fp_entry use LLVMSizeOfTypeInBits instead of 8 * LLVMStoreSizeOfType

https://github.com/gwenzek/zig/pull/4/files#r912686791

andrewrk added a commit that referenced this issue Jul 19, 2022
stage2 already has this fixed; debug info is given size in bits rather
than ABI size (bytes) multiplied by 8.

closes #11587
wooster0 pushed a commit to wooster0/zig that referenced this issue Jul 24, 2022
stage2 already has this fixed; debug info is given size in bits rather
than ABI size (bytes) multiplied by 8.

closes ziglang#11587
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend-llvm The LLVM backend outputs an LLVM IR Module. bug Observed behavior contradicts documented or intended behavior stage1 The process of building from source via WebAssembly and the C backend.
Projects
None yet
Development

No branches or pull requests

3 participants