eandrews marked 2 inline comments as done. eandrews added inline comments.
================ Comment at: clang/lib/AST/ASTContext.cpp:11496 + .getProgramAddressSpace() + : getTargetAddressSpace(T.getQualifiers()); +} ---------------- rjmccall wrote: > Oh, I'm sorry I missed this. Parsing the data layout string into an > `llvm::DataLayout` is definitely not an okay thing to be doing here. The > IRGen version of this had a cached `DataLayout` object which it queried, > which was okay, but this function is used too tightly to be doing that much > redundant work. > > We could just cache a `DataLayout` in the `clang::TargetInfo`, but I think > we've been trying to avoid that as a layering violation. Instead, > `TargetInfo` should just have a `getProgramAddressSpace` method or something > like that, and the targets with non-default address spaces for code should > set that manually. > Instead, `TargetInfo` should just have a `getProgramAddressSpace` method or > something like that, and the targets with non-default address spaces for code > should set that manually. Ok. Thanks for review! I'll make this change CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111566/new/ https://reviews.llvm.org/D111566 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
