[clang] [Clang][CodeGen] Emit load of GEP after EmitMemberExpr (PR #110487)
Bill Wendling via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 30 16:46:17 PDT 2024
bwendling wrote:
> This is sort of the same comment I've made on other related patches... but I think the fundamental issue here is that StructAccessBase is skipping over CK_LValueToRValue casts. Once you jump over such a cast, you're looking at expressions which are at a different level of indirection from the member access you care about.
I understand, but there are ~30 different types of implicit casts and it's hard to know which can be ignored and which shouldn't.
> If StructAccessBase just returns the cast itself, instead of trying to understand the value that's getting loaded, you shouldn't need a special case for MemberExpr.
I did an experiment and it has the same issue. If I return the LValueToRValue cast and run `EmitLValue` on it the 'load' is still missing.
In a separate PR, I wanted to create a MemberExpr based off of what the `StructAccessBase` returns, because all of the code to generate the correct access to that MemberExpr already exist, but you and Aaron had issues with that. I understand wanting to keep the AST in a sane state and not wanting to add nodes indiscriminately, but it's really limiting what can be done.
Here are two ways I'd like to handle this (in order of preference):
1. In CodeGen, when given and `Expr` in a `__builtin_dynamic_object_size` that has a FAM with `__counted_by`, I'd like to build a new `Expr` pointing to the *count* field and run that through the proper `Emit*()` method.
2. Convert a `__builtin_dynamic_object_size` with a FAM with `__counted_by` into the appropriate calculations in Sema.
For (1), building a new `Expr` is easier than emitting the "base" pointer to the structure and tacking on a reference to the count's `FieldDecl`, which has its own issues (like preventing us from loading the base pointer twice).
Option (2) isn't super awesome, because we lose the fact that the original expression was a `__builtin_dynamic_object_size` call. But maybe that's not important to keep around....
https://github.com/llvm/llvm-project/pull/110487
More information about the cfe-commits
mailing list