eddyz87 added inline comments.
================ Comment at: clang/lib/CodeGen/CGExpr.cpp:3791 + if (hasBPFPreserveStaticOffset(Base)) + addr = wrapWithBPFPreserveStaticOffset(CGF, addr); + ---------------- ast wrote: > 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. > If such cleanup makes sense at all. I think it does, I'll implement this change and push an update. 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