erichkeane added inline comments.
================
Comment at: clang/lib/CodeGen/CGExpr.cpp:3700
+ return false;
+ if (auto *BaseDecl = E->getType()->getPointeeType()->getAsRecordDecl())
+ return hasBPFPreserveStaticOffset(BaseDecl);
----------------
eddyz87 wrote:
> erichkeane wrote:
> > getPointeeType can also return nullptr, so unless you have a test elsewhere
> > to ensure it isn't, you likely have to do a little more work here (and if
> > so, I'd need an assert).
> Is it? I actually double-checked this before pushing an update, clangd jumps
> to the following definition:
>
> ```
> lang=cpp
> QualType Type::getPointeeType() const {
> if (const auto *PT = getAs<PointerType>())
> return PT->getPointeeType();
> ...
> return {};
> }
> ```
>
> The `getAsRecordDecl()` can return null indeed, but that null is checked.
More correctly, it returns an empty `QualType`. The `operator->` on that will
cause a `nullptr`, causing the call to `Type::getAsRecordDecl` to have a
`nullptr` `this`.
================
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}
----------------
eddyz87 wrote:
> erichkeane wrote:
> > Are we sure we want to do something like this? It seems this both depends
> > on YOUR computer AND us never releasing Clang 18.
> Are you sure this would be an issue?
> The specific line is not a part of a CHECK and I tried the following command
> using my system's llvm 16 opt:
>
> ```
> opt -O2 -mtriple=bpf-pc-linux -S -o - load-align.ll
> ```
>
> And module was loaded / processed w/o any issues.
> In general grepping shows that people don't usually mask these in tests:
>
> ```
> $ cd llvm/test/CodeGen/
> $ ag '{!"clang version' | wc -l
> 452
> ```
I don't write LLVM tests ever, so I'm not sure. It just seems odd to provide
that much irrelevant info, perhaps one of hte LLVM reviewers can comment.
Also, look at those ~450 and see what they contain?
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
