[PATCH] D90529: Allow nonnull/align attribute to accept poison

Tyker via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 15 08:20:58 PDT 2021


Tyker added a comment.

In D90529#2619492 <https://reviews.llvm.org/D90529#2619492>, @aqjune wrote:

> In D90529#2613303 <https://reviews.llvm.org/D90529#2613303>, @Tyker wrote:
>
>> In D90529#2612794 <https://reviews.llvm.org/D90529#2612794>, @aqjune wrote:
>>
>>> A fix in AssumeBundleBuilder to make it comply LangRef: D98228 <https://reviews.llvm.org/D98228>
>>>
>>> BTW, I found that "align" can take two operands: `"align"(i8* ptr, i64 a, i64 b)` What is the meaning of the second index (`b`)?
>>
>> the meaning of the second index is the offset of the alignment. so with `"align"(i8* ptr, i64 16, i64 12)`, ptr is aligned on 4 but ptr + 4 is aligned on 16.
>> I introduced this to support __builtin_assume_aligned which has similar semantics.
>
> Hi, thanks for the info.
>
> But I think an example in Transforms/AlignmentFromAssumptions/simple.ll conflicts with your definition. It has:
>
>   28 define i32 @foo2a(i32* nocapture %a) nounwind uwtable readonly {
>   29 entry:
>   30   tail call void @llvm.assume(i1 true) ["align"(i32* %a, i32 32, i32 28)]
>   31   %arrayidx = getelementptr inbounds i32, i32* %a, i64 -1
>   32   %0 = load i32, i32* %arrayidx, align 4
>   33   ret i32 %0
>   34 
>   35 ; CHECK-LABEL: @foo2a
>   36 ; CHECK: load i32, i32* {{[^,]+}}, align 32
>   37 ; CHECK: ret i32
>   38 }
>
> `"align"(i32* %a, i32 32, i32 28)` means `%a + 4` is 32-bytes aligned, IIUC. Then, `%a - 4` cannot be 32-bytes aligned, is it?

Yes, this looks like a bug.

> From https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html , the definition you've explained seems to be correct because it is saying
>
>   void *x = __builtin_assume_aligned (arg, 32, 8);
>   means that the compiler can assume for x, set to arg, that (char *) x - 8 is 32-byte aligned.
>
> Should we specify this in LangRef as well to make its definition explicit?

Yes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90529



More information about the llvm-commits mailing list