jyknight added inline comments.
================
Comment at: clang/lib/CodeGen/Address.h:26
llvm::Value *Pointer;
+ llvm::Type *PointeeType;
CharUnits Alignment;
----------------
erichkeane wrote:
> I think this will still end up being a problem when we try to look into the
> type for multi-dimension pointers. Should we have this just store the
> `QualType` of the value instead, so we can unpack it later? If we ever
> needed the `llvm::Type` of the Pointee, a `convertTypeForMem` is all that is
> standing in our way.
Do we do that? I didn't think Address was used that way.
================
Comment at: clang/lib/CodeGen/Address.h:29-30
public:
Address(llvm::Value *pointer, CharUnits alignment)
- : Pointer(pointer), Alignment(alignment) {
+ : Address(pointer, nullptr, alignment) {}
+ Address(llvm::Value *pointer, llvm::Type *PointeeType, CharUnits alignment)
----------------
nikic wrote:
> dblaikie wrote:
> > At some point will this include an assertion that 'pointer' isn't a
> > PointerType? I guess some uses of PointerTyped values won't need to know
> > their pointee type?
> >
> > (or are all values in Address PointerTyped? (owing to them being
> > "addresses"))?
> Based on the unconditional cast in getType(), I'd assume that addresses are
> always pointer typed.
This constructor seems odd. We shouldn't ever construct Address with a non-null
pointer and a null PointeeType, should we?
================
Comment at: clang/lib/CodeGen/Address.h:31
+ : Address(pointer, nullptr, alignment) {}
+ Address(llvm::Value *pointer, llvm::Type *PointeeType, CharUnits alignment)
+ : Pointer(pointer), PointeeType(PointeeType), Alignment(alignment) {
----------------
craig.topper wrote:
> Is PointeeType expected to be non-null when pointer is non-null?
That would be my expectation.
================
Comment at: clang/lib/CodeGen/Address.h:58
/// store it in Address instead for the convenience of writing code.
- llvm::Type *getElementType() const {
- return getType()->getElementType();
- }
+ llvm::Type *getElementType() const { return PointeeType; }
----------------
craig.topper wrote:
> Should this assert isValid() since it no longer goes through getType() and
> getPointer() which would have asserted previously?
+1.
================
Comment at: clang/lib/CodeGen/CGValue.h:230-231
private:
void Initialize(QualType Type, Qualifiers Quals, CharUnits Alignment,
LValueBaseInfo BaseInfo, TBAAAccessInfo TBAAInfo) {
assert((!Alignment.isZero() || Type->isIncompleteType()) &&
----------------
dblaikie wrote:
> Maybe this could use some assertions added to check the PointeeType is
> correct?
I think Initialize should be modified to take Addr instead of Alignment, and
move
```
R.V = Addr.getPointer();
R.PointeeType = Addr.getElementType();
```
into this function, from all the callers.
If that's done, I don't think a separate assert is useful.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103465/new/
https://reviews.llvm.org/D103465
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
