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
