mwyman added inline comments.
================ Comment at: clang/lib/CodeGen/CGObjC.cpp:1125 + llvm::Type *selType = CGF.ConvertType(CGF.getContext().getObjCSelType()); + return llvm::UndefValue::get(selType); + } ---------------- nlopes wrote: > Please consider using PoisonValue here instead (if appropriate). We are > trying to get rid of undef. > Thank you! > A poison value is a result of an erroneous operation. I could very well be misunderstanding, but based on this description from LangRef.html `PoisonValue` doesn't seem appropriate here; this is not "erroneous" behavior: it is just continuing the behavior prior to removing the `_cmd` implicit argument from the ABI, which was that the value was `undef` from the generated getter/setter's caller. https://github.com/llvm/llvm-project/blob/49acab3f1408be9439a57833aa7b67678c9a05ba/clang/lib/CodeGen/CGObjCMac.cpp#L2142 Since this is (essentially) replicating that behavior, I'm not sure `PoisonValue` is what we want. ================ Comment at: clang/test/CodeGenObjC/direct-method.m:178 +// CHECK-NEXT: [[IVAR:%.*]] = load {{.*}} @"OBJC_IVAR_$_Root._objectProperty", +// CHECK-NEXT: call i8* @objc_getProperty(i8* noundef [[SELF]], i8* noundef undef, i64 noundef [[IVAR]], {{.*}}) + ---------------- ahatanak wrote: > `noundef` means the value isn't undefined, right? Is it safe to use it with > `undef`? We might want to fix this if it isn't. It does feel questionable, however the pre-D131424 behavior was similar with the `undef` value coming from the caller and opaque to these generated getters/setters. At least according to the OSS drop of `libobjc`, nothing in `objc_getProperty`/`objc_setProperty` actually references the `_cmd` argument so it seems safe-ish for now: https://github.com/apple-oss-distributions/objc4/blob/8701d5672d3fd3cd817aeb84db1077aafe1a1604/runtime/objc-accessors.mm#L48 FWIW, searching the code, I do see a handful of `noundef undef` in some other tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135091/new/ https://reviews.llvm.org/D135091 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits