[PATCH] D110634: [llvm] Update IR verifier to reject non-power-of-2 alignment assume bundles (PR48713).

Sat Oct 2 11:27:13 PDT 2021

jdoerfert added a comment.

In D110634#3038161 <https://reviews.llvm.org/D110634#3038161>, @nikic wrote:

> In D110634#3028264 <https://reviews.llvm.org/D110634#3028264>, @jdoerfert wrote:
>
>> In D110634#3028153 <https://reviews.llvm.org/D110634#3028153>, @lebedev.ri wrote:
>>
>>> @jdoerfert wasn't the alignment intentionally been left as not being required to be constant?
>>
>> Yes. Though, I'm not even sure we emit/use non-constant alignments right now. If we emit them we would use them if they are proven to be constant later.
>> More elaborate support was eventually planned though. I would not require constants here, I overlooked that earlier.
>
> Well, if we don't require constants here then we can't add this check -- this means that some valid input IR (with non-constant non-power-of-two alignment) could become invalid after folding.

That is not wrong but also not new, I'd think. The verifier does local verification and after transformations things may be exposed that were missed before. It is always invalid for someone to put a non-power-of-two alignment somewhere and the only questions is when we identify that and reject it.

> Allowing non-constants here doesn't make a great deal of sense to me, in terms of practical usefulness. My understanding was that these operand bundles are supposed to model the corresponding metadata, and that doesn't allow non-constant values either.

That is true except that we have alignment source annotation that allows non-constant values. E.g., the alignment of the return is the second call argument to this function. If we don't allow non-constant alignments here we loose the ability to express these source annotations in IR.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110634