Skip to content

promote int8 to int32 if int8 is not supported#585

Open
39ali wants to merge 1 commit intoRust-GPU:mainfrom
39ali:u8-to-u32
Open

promote int8 to int32 if int8 is not supported#585
39ali wants to merge 1 commit intoRust-GPU:mainfrom
39ali:u8-to-u32

Conversation

@39ali
Copy link
Copy Markdown
Contributor

@39ali 39ali commented Apr 21, 2026

fixes #409

@39ali 39ali changed the title promote u8 to u32 if u8 is not supported promote int8 to int32 if u8 is not supported Apr 21, 2026
@39ali 39ali changed the title promote int8 to int32 if u8 is not supported promote int8 to int32 if int8 is not supported Apr 21, 2026
inst.operands[0] = Operand::LiteralBit32(32);
}

// fix OpConstant values: sign-extend signed 8-bit constants to 32 bits.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, you are only sign-extending signed integers and zero-extending unsigned ones

Copy link
Copy Markdown
Member

@Firestar99 Firestar99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incredibly dangerous, as you're promoting every single u8 to u32, regardless of where it is used. Imagine you're using OpIAddCarry and promoting that one, studdenly you're getting wrong results.

And if the single u8 delcaration is used anywhere in an OpTypePointer, even in another entry point unrelated to yours, you just back off and do nothing, meaning that unrelated code could affect whether this rewriting goes through or not.

Instead, you need to have a whitelist about what can be promoted, which is a huge rabbit hole. Feel free to read the discussion in #307 (which is linked from the issue you're trying to close).

@39ali
Copy link
Copy Markdown
Contributor Author

39ali commented Apr 24, 2026

oh wow thanks for the clarification, it's indeed a rabbit hole, i'll take a shot at addressing it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PartialOrd doesn't work with new type

2 participants