[PATCH] D106368: [AlignmentFromAssumptions] avoid crash on alignment constant expression

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D106368#2922582 <https://reviews.llvm.org/D106368#2922582>, @rmansfield wrote:

> In D106368#2890810 <https://reviews.llvm.org/D106368#2890810>, @lebedev.ri wrote:
>
>> In D106368#2890779 <https://reviews.llvm.org/D106368#2890779>, @spatel wrote:
>>
>>> In D106368#2890603 <https://reviews.llvm.org/D106368#2890603>, @jdoerfert wrote:
>>>
>>>> I still believe we should not create alignment annotations for non-power-of-two constants. This is the second time in a few weeks this causes problems and we still play whack-a-mole instead of restricting clang and teaching the verifier.
>>>>
>>>> https://reviews.llvm.org/D94433
>>>
>>> Ah, I didn't realize this is the same problem based on the reduced test. Someone familiar with this feature in Clang -- and compatibility with GCC? -- should have a look then ( @lebedev.ri @erichkeane ? ) - I have no idea what this is supposed to do.
>>
>> We (clang) already diagnoses it, but doesn't refuse to codegen it.
>> I think it should just hard-error on it, without emitting IR.
>
> Wouldn't that be revert of https://reviews.llvm.org/D73996 which softened it to a warning then?

Oh, i forgot that i had such a patch.
That seems to list a reason why clang does what it does,
I guess then we need two changes:

1. don't use alignment assume bundle to store non-power-of-two alignment, because that is obviously not an alignment
2. verifier to enforce that alignment assume bundle must contain powers of two.


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

https://reviews.llvm.org/D106368