[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

@jfb (and others), friendly ping. Are there any other changes (especially to the logic) that you'd like me to make?



================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1647
+                                        QualType Ty) {
+  auto *I8Ptr = CGF.Builder.CreateBitCast(Ptr, CGF.Int8PtrTy);
+  auto *Zero = ConstantInt::get(CGF.Int8Ty, 0);
----------------
jfb wrote:
> I'd like to hear @rsmith's thoughts on this approach, in particular w.r.t. aliasing concerns. I also wonder if below the GEPs should be inbounds, depending on how they're created.
Hmm, I don't know much about it but, I think these could be inbound. Because we will never actually go beyond the size of the llvm type. 


================
Comment at: clang/test/CodeGenCXX/builtin-zero-non-value-bits-codegen.cpp:46
+void test(Baz *baz) {
+  __builtin_zero_non_value_bits(baz);
+}
----------------
zoecarver wrote:
> zoecarver wrote:
> > jfb wrote:
> > > It would be useful to see a test for arrays with a type that contains tail padding.
> > Hmm, this test case doesn't seem to be working. I'll investigate further. 
> OK, I've added that. Just to clarify, you mean a type that contains a constant array type of types with tail padding (i.e., `Bar [2]`)?
Let me know if there are more codegen tests you'd like me to add. Happy to add them. (Maybe one for vector types, or volatile, or one that doesn't have an non-value bits?)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974