[PATCH] D138708: [SROA] Assert the AllocSize of i8 to be 1

Sat Nov 26 05:35:57 PST 2022

lebedev.ri added a comment.

In D138708#3951468 <https://reviews.llvm.org/D138708#3951468>, @jsilvanus wrote:

> I don't need support for overaligned `i8`s, it is fine for me to have LLVM require natural alignment on `i8`.
>
> But having explicit asserts maybe helps save some people's time in the future -- it certainly would have for me.
> Also, it explicitly documents the assumption underlying these GEPs.
>
> I'm not the sure the test brings additional value beyond motivating this change.
> There are probably many other places that can be broken by using such a data layout, and as long as 
> there is no effort to make overaligned `i8`s work I see little value in documenting these.
>
> Thus, I'd propose to not commit the test and just add the assertions.
> But if you feel this issue should be discussed further, I'm fine to open a thread on that.
>
> For what it's worth, I ran into a similar, separate issue with incorrect GEP offsets during SROA with vectors of overaligned elements that I *do* intend to fix, because that should work.

Thank you for looking into this.
I do agree that this looks like a problem that needs to be improved.
There is a caveat here: while we should assert liberally, reachable assertions should be fixed in some way.

Indeed, the problem appears to happen the moment we create a datalayout with overaligned byte type.
So let's just detect that there, diagnose it as invalid, and refuse to create it?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138708/new/

https://reviews.llvm.org/D138708