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

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

I went through LLVM part of the change, mostly look good to me from high-level and I need to go through another pass with details.
It would be great if you can post corresponding kernel patches which utilizes this functionality?



================
Comment at: clang/include/clang/Basic/AttrDocs.td:2227
+  void buz(struct bar *g) {
+    g->a.b = 42;
+  }
----------------
g->b.a ?


================
Comment at: llvm/lib/Target/BPF/BPFPreserveStaticOffset.cpp:82
+//   for (int i = 0; i < 4; ++i)
+//     foo(ctx->x[i]);
+//
----------------
Is it possible that such a pattern already changed by GVN/CSE so later on converting to preserve_static_offset becomes impossible? I guess it is unlikely but want to double check. Maybe add a comment to clarify.


================
Comment at: llvm/lib/Target/BPF/BPFPreserveStaticOffset.cpp:102
+// - the (get.and.load %p) would come from the callee function.
+// Thus clobbering CSE / GVN passes done after inlining.
+
----------------
So the above implies that this is bad for CSE/GVN due to different representation, right? But we want to do (1) since we want to avoid later CSE/GVN etc, so argument of clobbering is not that important, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133361