erichkeane added a comment.
The CFE stuff is pretty innocuous and I don't see a reason to stop this on our
accord, but would like someone familiar with the LLVM stuff to review this at
one point.
================
Comment at: clang/lib/CodeGen/CGExpr.cpp:3694
+static bool hasBPFPreserveStaticOffset(const RecordDecl *D) {
+ return D->getAttr<BPFPreserveStaticOffsetAttr>();
+}
----------------
================
Comment at: clang/lib/CodeGen/CGExpr.cpp:3698
+static bool hasBPFPreserveStaticOffset(const Expr *E) {
+ if (auto *PtrType = dyn_cast<PointerType>(E->getType()))
+ if (auto *BaseDecl = PtrType->getPointeeType()->getAsRecordDecl())
----------------
You can use `E->getType()->getPointeeType()` here instead, unless you REALLY
care that this is only a normal-pointer deref (and not a PMF or reference type).
================
Comment at: clang/lib/CodeGen/CGExpr.cpp:3778
+ if (Base && hasBPFPreserveStaticOffset(Base))
+ addr = wrapWithBPFPreserveStaticOffset(CGF, addr);
----------------
I'd suggest just making `hasBPFPreserveStaticOffset` be `nullptr` tolerant and
remove the 1st half of this.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7601
+ auto *Rec = cast<RecordDecl>(D);
+ Rec->addAttr(::new (S.Context) BPFPreserveStaticOffsetAttr(S.Context, AL));
+}
----------------
This should use `BPFPreserveStaticOffsetAttr::Create` instead of using
placement `new`. We've been meaning to translate these all at one point, but
never got around to it.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:8871
+ case ParsedAttr::AT_BPFPreserveStaticOffset:
+ handleBPFPreserveStaticOffsetAttr(S, D, AL);
+ break;
----------------
aaron.ballman wrote:
>
Or this, this is probably better.
================
Comment at: llvm/test/CodeGen/BPF/preserve-static-offset/load-align.ll:61
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{!"clang version 18.0.0 (/home/eddy/work/llvm-project/clang
c899a1ca75d0f1b559204eff79a2578d2cafc7ab)"}
+!2 = !{!3, !4, i64 128}
----------------
Are we sure we want to do something like this? It seems this both depends on
YOUR computer AND us never releasing Clang 18.
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
