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

bitstructs inside bitstructs #1977

Open
hypno2000 opened this issue Feb 16, 2025 · 9 comments
Open

bitstructs inside bitstructs #1977

hypno2000 opened this issue Feb 16, 2025 · 9 comments
Assignees
Labels
Enhancement Request New feature or request

Comments

@hypno2000
Copy link

hypno2000 commented Feb 16, 2025

Does it make sense allow bitstruct fields to be other bitstructs, as they are basically integers as well?

I'm working on Vulkan bindings where this would come handy:

def Flags  = uint;

bitstruct GeometryInstanceFlagsKHR : Flags @overlap {
	bool triangle_facing_cull_disable       : 0;
	bool triangle_flip_facing               : 1;
	bool force_opaque                       : 2;
	bool force_no_opaque                    : 3;
	bool force_opacity_micromap_2_state_ext : 4;
	bool disable_opacity_micromaps_ext      : 5;
	bool triangle_front_counterclockwise    : 1;
	bool triangle_cull_disable_nv           : 0;
	bool triangle_front_counterclockwise_nv : 1;
	bool force_opaque_nv                    : 2;
	bool force_no_opaque_nv                 : 3;
}

bitstruct AccelerationStructureMatrixMotionInstanceNVBits : uint {
	uint                     instanceShaderBindingTableRecordOffset : 0..23;
	GeometryInstanceFlagsKHR flags                                  : 24..31;
}

Right now i get this error:

Error: 'GeometryInstanceFlagsKHR' is not supported in a bitstruct, only enums, integer and boolean values may be used.

This is the related C header:

typedef enum VkGeometryInstanceFlagBitsKHR {
    VK_GEOMETRY_INSTANCE_TRIANGLE_FACING_CULL_DISABLE_BIT_KHR = 0x00000001,
    VK_GEOMETRY_INSTANCE_TRIANGLE_FLIP_FACING_BIT_KHR = 0x00000002,
    VK_GEOMETRY_INSTANCE_FORCE_OPAQUE_BIT_KHR = 0x00000004,
    VK_GEOMETRY_INSTANCE_FORCE_NO_OPAQUE_BIT_KHR = 0x00000008,
    VK_GEOMETRY_INSTANCE_FORCE_OPACITY_MICROMAP_2_STATE_EXT = 0x00000010,
    VK_GEOMETRY_INSTANCE_DISABLE_OPACITY_MICROMAPS_EXT = 0x00000020,
    VK_GEOMETRY_INSTANCE_TRIANGLE_FRONT_COUNTERCLOCKWISE_BIT_KHR = VK_GEOMETRY_INSTANCE_TRIANGLE_FLIP_FACING_BIT_KHR,
    VK_GEOMETRY_INSTANCE_TRIANGLE_CULL_DISABLE_BIT_NV = VK_GEOMETRY_INSTANCE_TRIANGLE_FACING_CULL_DISABLE_BIT_KHR,
    VK_GEOMETRY_INSTANCE_TRIANGLE_FRONT_COUNTERCLOCKWISE_BIT_NV = VK_GEOMETRY_INSTANCE_TRIANGLE_FRONT_COUNTERCLOCKWISE_BIT_KHR,
    VK_GEOMETRY_INSTANCE_FORCE_OPAQUE_BIT_NV = VK_GEOMETRY_INSTANCE_FORCE_OPAQUE_BIT_KHR,
    VK_GEOMETRY_INSTANCE_FORCE_NO_OPAQUE_BIT_NV = VK_GEOMETRY_INSTANCE_FORCE_NO_OPAQUE_BIT_KHR,
    VK_GEOMETRY_INSTANCE_FLAG_BITS_MAX_ENUM_KHR = 0x7FFFFFFF
} VkGeometryInstanceFlagBitsKHR;

typedef uint32_t VkFlags;
typedef VkFlags VkGeometryInstanceFlagsKHR;
typedef VkGeometryInstanceFlagsKHR VkGeometryInstanceFlagsNV;
typedef VkGeometryInstanceFlagBitsKHR VkGeometryInstanceFlagBitsNV;

typedef struct VkAccelerationStructureInstanceKHR {
    VkTransformMatrixKHR          transform;
    uint32_t                      instanceCustomIndex:24;
    uint32_t                      mask:8;
    uint32_t                      instanceShaderBindingTableRecordOffset:24;
    VkGeometryInstanceFlagsKHR    flags:8;
    uint64_t                      accelerationStructureReference;
} VkAccelerationStructureInstanceKHR;
hypno2000 added a commit to hypno2000/c3c that referenced this issue Feb 16, 2025
@lerno
Copy link
Collaborator

lerno commented Feb 16, 2025

You can use char[3] instead of uint that way it ought to fit?

@hypno2000
Copy link
Author

hypno2000 commented Feb 17, 2025

i think having uint as field type is fine as it should not use the field type for memory layout, it should just use the bit ranges for memory?

I could just switch field type from GeometryInstanceFlagsKHR to uint or even Flags and it would be fine but i'm trying to maintain GeometryInstanceFlagsKHR for API parity, typing and docs.

@lerno
Copy link
Collaborator

lerno commented Feb 17, 2025

It uses the field type for memory layout. If you use a uint on an LE system, then given bytes 0-3 in memory, byte 0 will have 0..7, byte 1 will have 8..15 and so on
On a BE system instead byte 0 will have 24..31, 1 will have 16..23 etc.

@hypno2000
Copy link
Author

hypno2000 commented Feb 17, 2025

Thanks, that makes sense. However, my issue is not related to memory layout or capacity constraints.

The correct type for this field is uint, as that is what Vulkan uses in the C API (uint32_t). To maintain binary compatibility with Vulkan, i need to use the same data types to ensure correct size, alignment, and endianness.

My issue is that C3 does not allow a bitstruct (which is of type uint) to be used as a field in another bitstruct.

Ideally, C3 should allow this, as bitstructs are fundamentally just integers with defined bit ranges, making them compatible with bitfields. Is there a technical reason why nested bitstructs are disallowed, or could this be considered?

@hypno2000
Copy link
Author

I noticed that i had left out the critical part from C header:

typedef uint32_t VkFlags;

i added that now. I hope it clears it up.

@hypno2000
Copy link
Author

hypno2000 commented Feb 18, 2025

probably makes sense to use anonymous bitstructs in this case, so the C3 translation is this:

def Flags = uint;

bitstruct GeometryInstanceFlagsKHR : Flags @overlap {
	bool triangle_facing_cull_disable       : 0;
	bool triangle_flip_facing               : 1;
	bool force_opaque                       : 2;
	bool force_no_opaque                    : 3;
	bool force_opacity_micromap_2_state_ext : 4;
	bool disable_opacity_micromaps_ext      : 5;
	bool triangle_front_counterclockwise    : 1;
	bool triangle_cull_disable_nv           : 0;
	bool triangle_front_counterclockwise_nv : 1;
	bool force_opaque_nv                    : 2;
	bool force_no_opaque_nv                 : 3;
}

struct AccelerationStructureInstanceKHR {
	TransformMatrixKHR transform;
	bitstruct : uint {
		uint instanceCustomIndex : 0..23;
		uint mask : 24..31;
	}
	bitstruct : uint {
		uint instanceShaderBindingTableRecordOffset : 0..23;
		GeometryInstanceFlagsKHR flags : 24..31;
	}
	ulong accelerationStructureReference;
}

Here i would like to keep using GeometryInstanceFlagsKHR instead of uint, same way as it is in C.

@lerno
Copy link
Collaborator

lerno commented Feb 18, 2025

Converting between bitstructs and bitfields with never be a 1:1 mapping, esp since C bitfields are different on different platforms. So keep in mind that this bitlayout may match on say Linux but is not guaranteed to work on Windows.

There are complexities to supporting flags in flags like you want, and even if it looks like you could make the compiler accept it, that's not the same as it actually is working. Note that unlike in C, you can perform various operations on bitstructs, and just having bitstructs in bitstructs together with @overlap is a lot of added complexity.

@hypno2000
Copy link
Author

Would it make sense for me to try to explore this a bit, ie make some test cases and see where it starts to break down, in which platform etc, and then we see if this is something the language wants to handle or not. Or you think it is probably a waste of time?

@lerno lerno self-assigned this Feb 24, 2025
@lerno lerno added the Enhancement Request New feature or request label Feb 24, 2025
@lerno
Copy link
Collaborator

lerno commented Mar 13, 2025

I am not really convinced this is needed. I think we looked at a variant with a union on discord. Didn't that resolve the issues?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Request New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants