Skip to content

Conversation

@s-barannikov
Copy link
Contributor

The change requires DataLayout instance to be available, which, in turn,
requires insertion point to be set. In-tree tests detected only one case
when the function was called without setting an insertion point, it was
changed to create a constant expression directly.

Copy link
Contributor Author

s-barannikov commented Aug 29, 2024

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

For this and #106540, I'm not convinced that we should actually make the canonical i8 type for ptradd byte width dependent. The only thing that matters is that the used type has an alloc size of one byte, so always using i8 should be fine.

I'd rather have the question "is this GEP a ptradd" be answerable without DL.

@s-barannikov
Copy link
Contributor Author

s-barannikov commented Aug 29, 2024

For this and #106540, I'm not convinced that we should actually make the canonical i8 type for ptradd byte width dependent. The only thing that matters is that the used type has an alloc size of one byte, so always using i8 should be fine.

Indeed, it is the type alloc size that matters most.
Sometimes, however, i8 geps lead to incorrect transformations or other unfortunate effects like infinite loops. In particular, this happens when geps are canonicalized: we check if the gep is a canonical, and if not, rewrite it to the canonical one, which is currently i8. Here is an example. We're changing i32 gep to i8 gep and multiplying the index by 4. With 32-bit bytes this transformation is incorrect because the canonical gep is i32 one, not i8.

I'd rather have the question "is this GEP a ptradd" be answerable without DL.

I'd like it too. I imagine the issue will resolve itself once we have ptradd as a first class instruction in LLVM IR rather than a gep pattern.

@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/3-bytewise-value branch from 3b86337 to a0d5c61 Compare October 27, 2024 00:57
@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/4-create-ptradd branch from 2a28c47 to ccdc8a1 Compare October 27, 2024 00:57
@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/3-bytewise-value branch from a0d5c61 to 9a1fad3 Compare October 27, 2024 01:34
@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/4-create-ptradd branch from ccdc8a1 to 4d3f0b6 Compare October 27, 2024 01:34
@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/3-bytewise-value branch from 9a1fad3 to 67765a6 Compare February 4, 2025 14:01
@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/4-create-ptradd branch from 4d3f0b6 to 6d95f98 Compare February 4, 2025 14:01
@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/3-bytewise-value branch from 67765a6 to 555e531 Compare April 6, 2025 17:22
@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/4-create-ptradd branch from 6d95f98 to b8c2041 Compare April 6, 2025 17:22
@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/3-bytewise-value branch from 555e531 to 3036dfc Compare April 19, 2025 08:18
@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/4-create-ptradd branch from b8c2041 to c335952 Compare April 19, 2025 08:19
@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/3-bytewise-value branch from 3036dfc to 81781bc Compare July 11, 2025 16:26
@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/4-create-ptradd branch from c335952 to 517a87c Compare July 11, 2025 16:26
@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/3-bytewise-value branch from 81781bc to 1f68188 Compare July 11, 2025 17:09
@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/4-create-ptradd branch from 517a87c to 1655f41 Compare July 11, 2025 17:09
@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/3-bytewise-value branch from 1f68188 to 4650b11 Compare July 11, 2025 17:12
@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/4-create-ptradd branch from 1655f41 to caa10b7 Compare July 11, 2025 17:12
The change requires DataLayout instance to be available, which, in turn,
requires insertion point to be set. In-tree tests detected only one case
when the function was called without setting an insertion point, it was
changed to create a constant expression directly.
@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/3-bytewise-value branch from 4650b11 to c5eb6f7 Compare November 8, 2025 09:11
@s-barannikov s-barannikov force-pushed the users/s-barannikov/byte/4-create-ptradd branch from caa10b7 to 29393ec Compare November 8, 2025 09:11
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.

3 participants