[PATCH] D135462: [SelectionDAG] Do not second-guess alignment for alloca

[Joseph Huber via Phabricator via llvm-commits]

jhuber6 added a subscriber: jdoerfert.
jhuber6 added a comment.

In D135462#4006016 <https://reviews.llvm.org/D135462#4006016>, @asavonic wrote:

> I checked OpenMP tests, and the difference is pretty much what was expected. This is what we have as an input (the patch does not change the IR):
>
>   // x86_64-pc-linux-gnu :: mapping/ompx_hold/struct.c
>   struct S {
>     int i;
>     int j;
>   } s;
>   // CHECK: presence of s, s.i, s.j: 0, 0, 0
>   CHECK_PRESENCE(s, s.i, s.j);
>
> IR:
>
>   %s = alloca %struct.S, align 4
>   %call = call i32 @omp_get_default_device()
>   %call1 = call i32 @omp_target_is_present(ptr noundef %s, i32 noundef %call)
>
> The assembly is different: we keep alignment of 4 and do not increase it to the preferred value of 8.
> Before the patch:
>
>   leaq    -16(%rbp), %rdi
>   callq   omp_target_is_present at PLT
>
> After the patch:
>
>   leaq    -12(%rbp), %rdi
>   callq   omp_target_is_present at PLT
>   
>   Libomptarget message: explicit extension not allowed: host address specified is 0x00007fff03109220 (12 bytes), but device allocation maps to host at 0x00007fff03109224 (8 bytes)
>
> If I explicitly set `__attribute__((aligned(8)))` to the declaration of `s`, assembly before and after the patch becomes equivalent, so the problem should go away. Apparently there is some discrepancy in alignment that the frontend (or the OpenMP runtime) expects, but it is not obvious where we need to fix this.

The error you get comes from the associated pointer we find extending before or after where we expect it to. My guess is that we think the device-side address is also four bytes, but is actually eight causing the discrepancy. I'm not sure why OpenMP seems to have a different preferred alignment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135462