[PATCH] D61173: [BPF] do not generate predefined macro bpf

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added inline comments.



Comment at: lib/Basic/Targets/BPF.cpp:23
  MacroBuilder &Builder) const {
-  DefineStd(Builder, "bpf", Opts);
+  Builder.defineMacro("__bpf");
+  Builder.defineMacro("__bpf__");

I don't think anyone is using this one.
May be we can keep ` __bpf__ ` and ` __BPF__ ` only?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61173



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61173: [BPF] do not generate predefined macro bpf

[Alexei Starovoitov via Phabricator via cfe-commits]

ast accepted this revision.
ast added a comment.
This revision is now accepted and ready to land.

shipit 



Repository:
  rC Clang

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

https://reviews.llvm.org/D61173



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65615: [BPF] annotate DIType metadata for builtin preseve_array_access_index()

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:3402
  QualType eltType, bool inbounds,
- bool signedIndices, SourceLocation loc,
+ bool signedIndices, QualType *arrayType,
+ SourceLocation loc,

would it make sense to reorder and make it 'QualType *arrayType = nullptr', so 
only explicit pointers would be passed and the rest will get 'nullptr' 
automatically?


Repository:
  rC Clang

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

https://reviews.llvm.org/D65615



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65615: [BPF] annotate DIType metadata for builtin preseve_array_access_index()

[Alexei Starovoitov via Phabricator via cfe-commits]

ast accepted this revision.
ast added a comment.
This revision is now accepted and ready to land.

and the diff is shorter now :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D65615



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

[Alexei Starovoitov via Phabricator via cfe-commits]

ast accepted this revision.
ast added a comment.
This revision is now accepted and ready to land.

from kernel, libbpf, tooling and user experience points of view this approach 
looks the best to me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

[Alexei Starovoitov via Phabricator via cfe-commits]

ast accepted this revision.
ast added a comment.

lgtm


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67734: [CLANG-BPF] change __builtin_preserve_access_index() signature

[Alexei Starovoitov via Phabricator via cfe-commits]

ast accepted this revision.
ast added a comment.
This revision is now accepted and ready to land.

that's indeed much better. Thanks for adding all the tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67734



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67883: [CLANG][BPF] permit any argument type for __builtin_preserve_access_index()

[Alexei Starovoitov via Phabricator via cfe-commits]

ast accepted this revision.
ast added a comment.
This revision is now accepted and ready to land.

I tested few different examples. Generated code and relocations look good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67883



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67980: [WIP][CLANG][BPF] do compile-once run-everywhere relocation for bitfields

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added inline comments.



Comment at: llvm/lib/Target/BPF/BPFCORE.h:17
+  enum OffsetRelocKind : uint32_t {
+FIELD_ACCESS_OFFSET = 0,
+FIELD_EXISTENCE,

why ACCESS_OFFSET is necessary?
Isn't it the same as BYTE_OFFSET but for non-bitfield?
May be single kind will do?



Comment at: llvm/lib/Target/BPF/BPFCORE.h:22
+FIELD_BYTE_OFFSET,
+FIELD_LSHIFT_64BIT_BUF,
+FIELD_RSHIFT_AFTER_LSHIFT_64BIT_BUF,

All these names are not added as builtin enum before compilation starts, right?
So C program cannot just use FIELD_BYTE_OFFSET and needs to define its own enum 
first?
How about FIELD_LSHIFT_U64 instead?



Comment at: llvm/lib/Target/BPF/BPFCORE.h:23
+FIELD_LSHIFT_64BIT_BUF,
+FIELD_RSHIFT_AFTER_LSHIFT_64BIT_BUF,
+

FIELD_RSHIFT_U64 ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67980



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67980: [CLANG][BPF] do compile-once run-everywhere relocation for bitfields

[Alexei Starovoitov via Phabricator via cfe-commits]

ast accepted this revision.
ast added a comment.
This revision is now accepted and ready to land.

lgtm. thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67980



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67980: [BPF] do compile-once run-everywhere relocation for bitfields

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

thanks for adding the tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67980



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67980: [BPF] do compile-once run-everywhere relocation for bitfields

[Alexei Starovoitov via Phabricator via cfe-commits]

ast accepted this revision.
ast added a comment.

perfect. ship it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67980



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69393: [RFC][DebugInfo] emit user specified address_space in dwarf

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

> The address spaces envisioned by the Linux kernel appear to be more 
> informational and not descriptive of hardware characteristics.

From the kernel pov the `__user` and normal are two different address spaces 
that must be accessed via different kernel primitives.
User access needs stac/clac on x86 and other precautions.
iirc other architectures have co-processor address space that needs its own 
load/store equivalents.
`__percpu` is also different address space. It's roughly equivalent to 
`__thread` in user space.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69393



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81479: [BPF] introduce __builtin_load_u32_to_ptr() intrinsic

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added inline comments.



Comment at: clang/test/CodeGen/builtin-bpf-load-u32-to-ptr.c:5
+struct t { int a; int b; };
+void *test(struct t *arg) { return __builtin_load_u32_to_ptr(arg, 4); }
+

can it be expressed as:
__builtin_load_u32_to_ptr(&arg->b) ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81479



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81479: [BPF] introduce __builtin_bpf_load_u32_to_ptr() intrinsic

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

It feels that the same thing can be represented as inline asm.
What advantage builtin has?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81479



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83242: [clang][BPF] support expr with typedef/record type for FIELD_EXISTENCE reloc

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

lgtm.
curious what happens when type is defined within args, like:
__builtin_preserve_field_info(*(struct { int a; } *)0, 2);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83242



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85174: BPF: simplify IR generation for __builtin_btf_type_id()

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

Is it a cleanup or is it a fix for some bug? If latter there should be a new 
test for it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85174

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93103: Enable the _ExtInt extension on the BPF Target

[Alexei Starovoitov via Phabricator via cfe-commits]

ast requested changes to this revision.
ast added a comment.
This revision now requires changes to proceed.

What's a use case?
The test is necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93103

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72184: [BPF] support atomic instructions

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added inline comments.



Comment at: llvm/test/CodeGen/BPF/atomics_2.ll:124
+; CHECK: r0 = r2
+; CHECK: r0 = cmpxchg_64(r1 + 0, r0, r3)
+; CHECK: encoding: [0xdb,0x01,0x00,0x00,0xf1,0x03,0x00,0x00]

Looks like it's generating correct code without special handling in 
ISelLowering. Nice. I wonder why x86 had to use it. Because of eflags, I guess?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91489: BPF: make __builtin_btf_type_id() return 64bit int

[Alexei Starovoitov via Phabricator via cfe-commits]

ast accepted this revision.
ast added inline comments.
This revision is now accepted and ready to land.



Comment at: llvm/lib/Target/BPF/BTFDebug.cpp:1230
+Reloc == BPFCoreSharedInfo::BTF_TYPE_ID_REMOTE)
   OutMI.setOpcode(BPF::LD_imm64);
 else

libbpf would need to support both (ld_imm64 and mov32), right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91489

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72184: [BPF] support atomic instructions

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added inline comments.



Comment at: llvm/lib/Target/BPF/BPFInstrInfo.td:684
+  let Inst{47-32} = addr{15-0}; // offset
+  let Inst{11-8} = val;
+  let Inst{7-4} = Opc.Value;

jackmanb wrote:
> yonghong-song wrote:
> > jackmanb wrote:
> > > jackmanb wrote:
> > > > jackmanb wrote:
> > > > > Sorry I'm a beginner with the LLVM code, could you explain what `val` 
> > > > > does? I didn't notice this when I looked through here before.
> > > > > 
> > > > > To try and get a clue I tried just removing this line and then 
> > > > > compiling the following code:
> > > > > 
> > > > > ```C
> > > > > // SPDX-License-Identifier: GPL-2.0
> > > > > #include 
> > > > > 
> > > > > #include 
> > > > > #include 
> > > > > #include 
> > > > > 
> > > > > __u64 test_data_64 = 0;
> > > > > __u64 test1_result = 0;
> > > > > 
> > > > > SEC("fentry/bpf_fentry_test1")
> > > > > int BPF_PROG(test1, int a)
> > > > > {
> > > > > /* atomic_fetch_add(&test_data_64, 1); */
> > > > > test1_result = __sync_fetch_and_add(&test_data_64, 1);
> > > > > return 0;
> > > > > }
> > > > > ```
> > > > > 
> > > > > And I was able to load and run the program, with the kernel on my WIP 
> > > > > branch: https://github.com/bjackman/linux-bpf/tree/wips/bpf-atomics-v0
> > > > > 
> > > > > The result looks like this:
> > > > > 
> > > > > ```shell
> > > > > $ llvm-objdump -d atomics_test.o
> > > > > 
> > > > > atomics_test.o: file format elf64-bpf
> > > > > 
> > > > > 
> > > > > Disassembly of section fentry/bpf_fentry_test1:
> > > > > 
> > > > >  :
> > > > >0:   b7 01 00 00 01 00 00 00 r1 = 1
> > > > >1:   18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 
> > > > > 0 ll
> > > > >3:   db 12 00 00 01 00 00 00 r1 = atomic_fetch_add((u64 
> > > > > *)(r2 + 0), PLEASE submit a bug report to https://bugs.llvm.org/ and 
> > > > > include the crash backtrace.
> > > > > Stack dump:
> > > > > 0.  Program arguments: llvm-objdump -d atomics_test.o 
> > > > > Segmentation fault
> > > > > ```
> > > > > 
> > > > > Aside from the fact that llvm-objdump crashed, the encoding `db 12 00 
> > > > > 00 01 00 00 00` seems correct to me. If I add the `let Inst{11-8} = 
> > > > > val` back in I get `db 12 00 00 01 01 00 00` which I don't understand.
> > > > Ah wait, I guess this is adding a 3rd operand register? In my example 
> > > > it's unclear because it is R1 which is also the dst operand. I was 
> > > > envisaging we just have semantics like `src = atomic_fetch_add(dst+off, 
> > > > src)` but you are instead proposing `dst = atomic_fetch_add(dst+off, 
> > > > val)`?
> > > Sorry I mean `dst = atomic_fetch_add(src+off, val)`
> > > Sorry I'm a beginner with the LLVM code, could you explain what `val` 
> > > does? I didn't notice this when I looked through here before.
> > 
> > For the below statement:
> >   test1_result = __sync_fetch_and_add(&test_data_64, 1);
> > 
> > 'val' represents the register which holds the value '1'.
> > 
> > bit 4-7 is also used in compare-and-exchange insn as you need a memory 
> > location, in-register old/new values.
> > 
> > > 
> > > To try and get a clue I tried just removing this line and then compiling 
> > > the following code:
> > > 
> > > ```C
> > > // SPDX-License-Identifier: GPL-2.0
> > > #include 
> > > 
> > > #include 
> > > #include 
> > > #include 
> > > 
> > > __u64 test_data_64 = 0;
> > > __u64 test1_result = 0;
> > > 
> > > SEC("fentry/bpf_fentry_test1")
> > > int BPF_PROG(test1, int a)
> > > {
> > > /* atomic_fetch_add(&test_data_64, 1); */
> > > test1_result = __sync_fetch_and_add(&test_data_64, 1);
> > > return 0;
> > > }
> > > ```
> > > 
> > > And I was able to load and run the program, with the kernel on my WIP 
> > > branch: https://github.com/bjackman/linux-bpf/tree/wips/bpf-atomics-v0
> > > 
> > > The result looks like this:
> > > 
> > > ```shell
> > > $ llvm-objdump -d atomics_test.o
> > > 
> > > atomics_test.o: file format elf64-bpf
> > > 
> > > 
> > > Disassembly of section fentry/bpf_fentry_test1:
> > > 
> > >  :
> > >0:   b7 01 00 00 01 00 00 00 r1 = 1
> > >1:   18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0 ll
> > >3:   db 12 00 00 01 00 00 00 r1 = atomic_fetch_add((u64 *)(r2 
> > > + 0), PLEASE submit a bug report to https://bugs.llvm.org/ and include 
> > > the crash backtrace.
> > > Stack dump:
> > > 0.  Program arguments: llvm-objdump -d atomics_test.o 
> > > Segmentation fault
> > > ```
> > > 
> > 
> > the crash may come from that the 'val' is not encoded properly. I will 
> > double check.
> > 
> > > Aside from the fact that llvm-objdump crashed, the encoding `db 12 00 00 
> > > 01 00 00 00` seems correct to me. If I add the `let Inst{11-8} = val` 
> > > back in I get `db 12 00 00 01 01 00 00` which I don't understand.
> > 
> > in this particular case, yes, as final asm code looks like
> >r1 = atomic_fet

[PATCH] D72184: [BPF] support atomic instructions

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

looks good. Before landing we need to agree on the full set of instructions 
that -mcpu=v4 will support.
atomic_fetch_or|xor|and are probably needed as instructions. The kernel JIT 
will generate x86 cmpxchg for them.
Because if llvm generates bpf cmpxchg insn then we'd need to teach the verifier 
to recognize infinite loops.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72184: [BPF] support atomic instructions

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added inline comments.



Comment at: llvm/lib/Target/BPF/BPFInstrInfo.td:783
+  let Inst{47-32} = addr{15-0}; // offset
+  let Inst{11-8} = new;
+  let Inst{7-4} = BPF_CMPXCHG.Value;

yonghong-song wrote:
> jackmanb wrote:
> > If we go down the route of matching operands with x86 as you have done for 
> > `XFALU64` and `XCHG`, I think  we should also do it for cmpxchg.
> > 
> > IIUC this is `dst = atomic_cmpxchg(*(src + off), r0, new);` 
> > 
> > But to do it in a single x86 instruction we need to have only 2 operands + 
> > the hard-coded r0. `r0 = atomic_xmpxchg(*(dst + off), r0, src);` would seem 
> > most natural to me.
> We can do this:
>   r0 = atomic_xmpxchg(*(dst + off), r0, src);
I'm confused. Isn't that what that is already?
r0 = atomic_cmpxchg(*(dst + off), r0, src);

In "class CMPXCHG":
let Inst{51-48} = addr{19-16}; // this is 'dst_reg' in bpf_insn.
let Inst{55-52} = new; // this is 'src_reg' in bpf_insn.

Same as old xadd and new fetch_add insns.
What am I missing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72184: [BPF] support atomic instructions

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added inline comments.



Comment at: llvm/test/CodeGen/BPF/atomics_2.ll:14
+;   int test_xchg_64(long *p, long v) {
+; return __sync_lock_test_and_set(p, v);
+;   }

test_and_set is not the same as xchg.
xchg doesn't do comparison.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72184: [BPF] support atomic instructions

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added inline comments.



Comment at: llvm/test/CodeGen/BPF/atomics_2.ll:14
+;   int test_xchg_64(long *p, long v) {
+; return __sync_lock_test_and_set(p, v);
+;   }

yonghong-song wrote:
> ast wrote:
> > test_and_set is not the same as xchg.
> > xchg doesn't do comparison.
> I am looking at here:
>   https://gcc.gnu.org/onlinedocs/gcc-4.1.1/gcc/Atomic-Builtins.html
> which mentions:
>  type __sync_lock_test_and_set (type *ptr, type value, ...)
> This builtin, as described by Intel, is not a traditional test-and-set 
> operation, but rather an atomic exchange operation. It writes value into 
> *ptr, and returns the previous contents of *ptr.
> 
> Many targets have only minimal support for such locks, and do not support 
> a full exchange operation. In this case, a target may support reduced 
> functionality here by which the only valid value to store is the immediate 
> constant 1. The exact value actually stored in *ptr is implementation defined.
> 
> This builtin is not a full barrier, but rather an acquire barrier. This 
> means that references after the builtin cannot move to (or be speculated to) 
> before the builtin, but previous memory stores may not be globally visible 
> yet, and previous memory loads may not yet be satisfied. 
> 
> So it does not do compare.
> 
> Or alternatively for llvm atomic builtin,
>   https://llvm.org/docs/Atomics.html
> 
> We have:
>   iN __atomic_load_N(iN *ptr, iN val, int ordering)
> void __atomic_store_N(iN *ptr, iN val, int ordering)
> iN __atomic_exchange_N(iN *ptr, iN val, int ordering)
> bool __atomic_compare_exchange_N(iN *ptr, iN *expected, iN desired, int 
> success_order, int failure_order)
> 
> But as I mentioned in bpf office hour meeting, a "ordering" is required and I 
> do not know how to deal with it.
Thanks for the info. gcc builtin has a misleading name.
Please mention this quirk in the tests comment and in the commit log.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72184: [BPF] support atomic instructions

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

Looks like the test didn't change. Only commit log... that's fine. I think the 
diff is ready to land, but let's wait for the kernel patches to be ready as 
well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72184: [BPF] support atomic instructions

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added inline comments.



Comment at: llvm/lib/Target/BPF/BPFMIChecking.cpp:199
+  unsigned newOpcode;
+  switch(MI.getOpcode()) {
+  case BPF::XFADDW32: newOpcode = BPF::XADDW32; break;

With this logic in place Andrii has a point. There is no need for -mcpu=v4 flag.
The builtins will produce new insns and it will be up to kernel to accept them 
or not.



Comment at: llvm/lib/Target/BPF/BPFMIChecking.cpp:207
+  case BPF::XFORW32: newOpcode = BPF::XORW32; break;
+  default: newOpcode = BPF::XORD; break;
+  }

The default is error prone. Why not to check for XFORD explicitly and default 
to fatal_error?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72184: [BPF] support atomic instructions

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added inline comments.



Comment at: llvm/lib/Target/BPF/BPFMIChecking.cpp:199
+  unsigned newOpcode;
+  switch(MI.getOpcode()) {
+  case BPF::XFADDW32: newOpcode = BPF::XADDW32; break;

yonghong-song wrote:
> yonghong-song wrote:
> > ast wrote:
> > > With this logic in place Andrii has a point. There is no need for 
> > > -mcpu=v4 flag.
> > > The builtins will produce new insns and it will be up to kernel to accept 
> > > them or not.
> > will make the change (removing -mcpu=v4).
> There will be one user-level change:
>   . if user has "ret = __sync_fetch_and_add(p, r);", today's llvm will 
> generate a fatal error saying that invalid XADD since return value is used.
>   . but removing -mcpu=v4. the compilation will be successful and the error 
> will happen in kernel since atomic_fetch_add is not supported.
> 
> I guess this is an okay change, right?
right. With new llvm compilation will succeed, but it will fail to load on 
older kernel and will work as expected on newer kernel. There is a change where 
the error will be seen, but that's acceptable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72184: [WIP][BPF] support exchange/compare-and-exchange instruction

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added inline comments.



Comment at: llvm/lib/Target/BPF/BPFInstrInfo.td:715
+
+let Predicates = [BPFHasAtomicExt, BPFHasALU32], DecoderNamespace = "BPFALU32" 
in {
+  def XFADDW32 : XFALU32;

i think -mcpu=v4 should include alu32.
Otherwise the test matrix will keep increasing. It's already time consuming to 
test v1,v2,v3.
If v4 would mean with and without alu32 that would stay this way for long time 
and any further extensions
would be doubling the test matrix further.



Comment at: llvm/lib/Target/BPF/BPFInstrInfo.td:830
+
+let Predicates = [BPFHasAtomicExt] in {
+  def CMPXCHGD : CMPXCHG;

let Defs = [R0], Uses = [R0]
and BPFISelLowering would need to do getCopyToReg+cmpxchg+getCopyFromReg 
similar to X86ISelLowering ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103549: [POC] Put annotation strings into debuginfo.

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

> Yep LLVM does have some custom attributes and such, eg: 
> https://github.com/llvm/llvm-project/blob/effb87dfa810a28e763f914fe3e6e984782cc846/llvm/include/llvm/BinaryFormat/Dwarf.def#L592

This is great insight! That should work. We need to make sure that 
DW_AT_LLVM_annotation in structure/field/func/argument/variable won't confuse 
gdb and such.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103549

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132144: [Clang][BPF] Support record argument with direct values

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

clang -target bpf -O2 -S t3.c
t3.c:1:5: error: defined with too many args

this will be the case when backend needs to emit the code,
but if the function is always_inline it can have more than 5 arguments, right?
Would be good to have a test for that to make sure when BPF_KPROBE macro 
supports casting of 16-byte structs the clang side won't be an issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132144

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142046: [BPF][clang] Ignore stack protector options for BPF target

[Alexei Starovoitov via Phabricator via cfe-commits]

ast accepted this revision.
ast added a comment.
This revision is now accepted and ready to land.

This approach looks better to me than NVPTX warn and I agree with Ed that it's 
better to leave NVPTX as-is to avoid any regression.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142046

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142046: [BPF][clang] Ignore stack protector options for BPF target

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

In D142046#4066758 , @yonghong-song 
wrote:

> @ast With this patch, gentoo clang compilation will hit the warning even if 
> people appends -fno-stack-protector. Is this okay? In general, if the option 
> is '-fstack-protector -fno-stack-protector', we should not issue warning, 
> right?

ahh. yeah. -fno-stack-protector should silence the warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142046

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144829: [WIP][BPF] Add a few new insns under cpu=v4

[Alexei Starovoitov via Phabricator via cfe-commits]

ast accepted this revision.
ast added a comment.
This revision is now accepted and ready to land.

overall looks good. one small nit.




Comment at: llvm/lib/Target/BPF/MCTargetDesc/BPFMCFixups.h:17
+enum FixupKind {
+  // These correspond directly to R_390_* relocations.
+  FK_BPF_PCRel_4 = FirstTargetFixupKind,

a little bit too much of copy paste :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144829

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144829: [BPF] Add a few new insns under cpu=v4

[Alexei Starovoitov via Phabricator via cfe-commits]

ast accepted this revision.
ast added a comment.

lgtm. @eddyz87 pls take a look


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144829

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133361: [BPF] Attribute preserve_static_offset for structs

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:3791
+  if (hasBPFPreserveStaticOffset(Base))
+addr = wrapWithBPFPreserveStaticOffset(CGF, addr);
+

If I'm reading this correctly wrapping with preserve_static_offset doesn't 
prevent further preserver_access_index wrapping which is a wasted effort for 
pai at the end ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133361

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133361: [BPF] Attribute preserve_static_offset for structs

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:3791
+  if (hasBPFPreserveStaticOffset(Base))
+addr = wrapWithBPFPreserveStaticOffset(CGF, addr);
+

eddyz87 wrote:
> ast wrote:
> > If I'm reading this correctly wrapping with preserve_static_offset doesn't 
> > prevent further preserver_access_index wrapping which is a wasted effort 
> > for pai at the end ?
> Yes, pai calls are undone in `BPFPreserveStaticOffset.cpp:removePAICalls()`. 
> I can put back the logic that suppresses pai if preserve static offset is 
> present.
I see. I guess I missed a previous discussion. Why this approach was chosen?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133361

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133361: [BPF] Attribute preserve_static_offset for structs

[Alexei Starovoitov via Phabricator via cfe-commits]

ast accepted this revision.
ast added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:3791
+  if (hasBPFPreserveStaticOffset(Base))
+addr = wrapWithBPFPreserveStaticOffset(CGF, addr);
+

eddyz87 wrote:
> ast wrote:
> > eddyz87 wrote:
> > > ast wrote:
> > > > If I'm reading this correctly wrapping with preserve_static_offset 
> > > > doesn't prevent further preserver_access_index wrapping which is a 
> > > > wasted effort for pai at the end ?
> > > Yes, pai calls are undone in 
> > > `BPFPreserveStaticOffset.cpp:removePAICalls()`. I can put back the logic 
> > > that suppresses pai if preserve static offset is present.
> > I see. I guess I missed a previous discussion. Why this approach was chosen?
> Initial version used `__attribute__((btf_decl_tag("ctx")))` and Yonghong did 
> not want to have prioritization between `btf_decl_tag` and 
> `preserve_access_index` basing on decl tag string parameter. Now this 
> limitation is gone (and I think this was one of your arguments in favor of 
> separate attribute).
Ahh. Right. It made sense to avoid special treatment of strings in decl_tag, 
but now it's gone and PSO takes precedence over PAI. Here we're adding PAI just 
to remove it later. Looks like a waste of cpu cycles and code. Unless applying 
PAI to outer struct and PSO in inner makes implementation tricky. I doubt we 
need to support such combo though. I'm fine cleaning this up in a follow up. If 
such cleanup makes sense at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133361

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144829: [WIP][BPF] Add a few new insns under cpu=v4

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added inline comments.



Comment at: llvm/lib/Target/BPF/BPFInstrInfo.td:56
 def BPFNoALU32 : Predicate<"!Subtarget->getHasAlu32()">;
+def BPFHasCPUv4_ldsx : Predicate<"Subtarget->getCPUv4_ldsx()">;
+def BPFHasCPUv4_movsx : Predicate<"Subtarget->getCPUv4_movsx()">;

Here and elsewhere... let's drop CPUv4 mid prefix. imo the extra verbosity 
doesn't improve readability.
Same with the flag: disable-cpuv4-movsx. I can be disable-movsx.

s/BPFHasCPUv4_ldsx/BPFHasLdsx/
s/getCPUv4_bswap/getHasBswap/ or even shorter hasBswap ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144829

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133361: [BPF] Attribute btf_decl_tag("ctx") for structs

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

> I can rename these things, but tbh I don't think this functionality would be 
> useful anywhere outside BPF, thus such renaming would be kind-of deceptive 
> (and in case it would be useful, the renaming could be done at the time of 
> second use).

I agree that it's not useful outside of BPF, but it's useful outside of 'ctx'. 
I think 'preserve_constant_field_offset' would be more accurate description of 
the restriction.
We can expand in the doc that it's a constant offset when field of the struct 
is accessed.

Also instead of btf_tag it would be better to add another builtin similar to 
preserve_access_index.
Currently we add __attribute__((preserve_access_index)) to trigger CO-RE.
This one will be a new __attribute__((preserve_constant_field_offset)) that 
will be specified manually either in uapi/bpf.h or in vmlinux.h on some structs
and it will have precedence over preserve_access_index, so
(__attribute__((preserve_access_index)), apply_to = record) in vmlinux.h will 
be ignored on such structs.
Otherwise it's a bit odd that special names inside btf_tag have stronger rules 
than other __attribute__((preserve_access_index)).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133361

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133361: [BPF] Attribute btf_decl_tag("ctx") for structs

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

In D133361#4629081 , @eddyz87 wrote:

> Note on current propagation logic: whenever there is an expression E of type 
> T, where T is a struct annotated with btf_decl_tag("ctx"), all usages of  E 
> are traversed recursively visiting `getelementptr` and calls to 
> `preserve_{struct,union,array}_index`, the latter are replaced by 
> `getelementptr`. E.g.:
>
>   #define __pai __attribute__((preserve_access_context));
>   #define __ctx __attribute__((btf_decl_tag("ctx")))
>   struct bar { int a; int b; } __pai;
>   struct foo {
> struct bar b;
>   } __ctx __pai;
>   ... struct foo f; ...
>   ... f.b.bb ...
>
> The call to `preserve_struct_index` generated for `bb` in `f.b.bb` would be 
> replaced by `getelementptr` and later by `getelementptr.and.load`.
> However, context structures don't have nested structures at the moment. The 
> list of context structures is:
>
> - __sk_buff
> - bpf_cgroup_dev_ctx
> - bpf_sk_lookup
> - bpf_sock
> - bpf_sock_addr
> - bpf_sock_ops
> - bpf_sockopt
> - bpf_sysctl
> - sk_msg_md
> - sk_reuseport_md
> - xdp_md

Right. Such recursive propagation of PAI is necessary. For btf_tag we cannot do 
it. Always propagating it won't be correct.
New preserve_const_field_offset would need to be propagated too and we actually 
have nested unions.
__bpf_md_ptr is such example. btf_tag wouldn't propagate into that union, but 
attr(preserve_const_field_offset) should.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133361

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133361: [BPF] Attribute btf_decl_tag("ctx") for structs

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

In D133361#4637008 , @eddyz87 wrote:

> In D133361#4629292 , @ast wrote:
>
>> ...
>> Right. Such recursive propagation of PAI is necessary. For btf_tag we cannot 
>> do it. Always propagating it won't be correct.
>> New preserve_const_field_offset would need to be propagated too and we 
>> actually have nested unions.
>> __bpf_md_ptr is such example. btf_tag wouldn't propagate into that union, 
>> but attr(preserve_const_field_offset) should.
>
> Hi Alexei,
>
> It just occurred to me that such an attribute would also require DWARF and 
> BTF encoding in order to get reflected in vmlinux.h (which we already have 
> for btf_decl_tag). Given this I think we can rename decl tag "ctx" to 
> `btf_decl_tag("preserve_const_field_offset")` but we should still keep it a 
> `btf_decl_tag`. I'll try to replace usage of `bpf_context_marker` intrinsic 
> by metadata, if that fails will just rename the intrinsic to 
> `preserve_const_field_offset`.
> What do you think? (Sorry, I should have thought this through last week).

I still feel that new attr is much cleaner from llvm implementation/design 
perspective and vmlinux.h inconvenience should be low priority in this 
considerations.
Since ctx only applies to uapi/bpf.h header the users don't have to use 
vmlinux.h. I know that today we have pains combining uapi headers and vmlinux.h 
and several solutions were
proposed. None were accepted yet, but that shouldn't mean that we should 
sacrifice llvm implementation due to orthogonal issues.
As a temporary workaround for vmlinux.h we can have uapi/bpf.h to do 
attr(preserve_const_field_offset) _and_ btf_decl_tag("bpf_ctx_struct"). Then 
teach pahole to emit attr(preserve_const_field_offset)
when it sees btf_decl_tag("bpf_ctx_struct") in vmlinux BTF.
Other workarounds are possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133361

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74668: [Clang][BPF] implement __builtin_btf_type_id() builtin function

[Alexei Starovoitov via Phabricator via cfe-commits]

ast accepted this revision.
ast added a comment.
This revision is now accepted and ready to land.

lgtm. Thanks for explaining lvalue/value trick.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74668



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74572: [BPF] preserve debuginfo types for builtin __builtin__btf_type_id()

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added inline comments.



Comment at: llvm/lib/Target/BPF/BPFPreserveDIType.cpp:1
+//===- BPFPreserveType.cpp - Preserve DebugInfo Types 
-===//
+//

BPFPreserveDIType.cpp



Comment at: llvm/lib/Target/BPF/BPFPreserveDIType.cpp:95
+
+  std::string BaseName = "llvm.bpf_pdit.";
+  int Count = 0;

may be "llvm.btf_type_id." instead?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74572: [BPF] preserve debuginfo types for builtin __builtin__btf_type_id()

[Alexei Starovoitov via Phabricator via cfe-commits]

ast accepted this revision.
ast added inline comments.
This revision is now accepted and ready to land.



Comment at: llvm/lib/Target/BPF/BPFPreserveDIType.cpp:1
+//===- BPFPreserveType.cpp - Preserve DebugInfo Types 
-===//
+//

ast wrote:
> BPFPreserveDIType.cpp
this typo wasn't fixed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69759: [BPF] Add preserve_access_index attribute for record definition

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

Looks great. This is a big improvement in usability.
Is there a use case to apply that attribute to inner types automatically ?

  struct s1 {
int foo;
  };
  struct s2 {
 struct s1 *ptr;
  } __reloc__ *s2;

s2->ptr->foo -- will both deref be relocatable or only first?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69759



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69759: [BPF] Add preserve_access_index attribute for record definition

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

Is the attribute sticky with forward delcarations?

  struct s __reloc;
  
  struct s {
int foo;
  } *s;

what is s->foo ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69759



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69759: [BPF] Add preserve_access_index attribute for record definition

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

So what happens in the following case:

  struct S1;
  struct S2 {
struct S1 *f;
  } relo *s2;
  // access s2->f
  struct S1 {
int i;
  } relo;
  // access s2->f->i

lack of relo on fwd declaration is not going to be sticky 'non-relo' when it's 
seen later, right?
So all 3 deref will be relocatable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69759



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69759: [BPF] Add preserve_access_index attribute for record definition

[Alexei Starovoitov via Phabricator via cfe-commits]

ast accepted this revision.
ast added a comment.
This revision is now accepted and ready to land.

Great. Looks like inner propagation fix was straighforward. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69759



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69759: [BPF] Add preserve_access_index attribute for record definition

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added a comment.

I've tested this patch on several kernel selftests/bpf/ and it works like 
magic. Very nice improvement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69759



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

[Alexei Starovoitov via Phabricator via cfe-commits]

ast accepted this revision.
ast added a comment.
This revision is now accepted and ready to land.

Makes sense to me. Only BPF target will notice this difference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100567

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits