[PATCH] D82316: [LangRef] Add `frozen` attribute to documentation

Mon Jun 22 17:12:50 PDT 2020

eugenis added inline comments.

================
Comment at: llvm/docs/LangRef.rst:1259
+    requirement include structures with padding bits, unions with active
+    members smaller than their size, and scalars which were never initialized.
+
----------------
aqjune wrote:
> nikic wrote:
> > This description is a bit too frontend-centric. I'd suggest to at least add
> > 
> > > If the parameter is poison or contains undefined bits, the behavior is undefined.
> > 
> > to describe the actual IR semantic this has.
> > 
> > Also, can this be applied to return values? If so, it should say so.
> For aggregate types, I think it is okay to relax this constraint to allowing aggregates with undef padding bits. It will be great if an aggregate value that is constructed with a chain of `v = insertvalue (insertvalue undef, const1, idx), const2, idx2` can have this attribute attached regardless of whether the aggregate type has paddings or not. Inspecting padding bits which are not accessible without using memory operation may be overly restrictive.
> For my previous concern (giving a guarantee that it has no undef bit in memory representation), this should be needed in certain future cases, but not now maybe.
Yes, that's a good point. From MSan point of view, if {i8, i32} is passed as a call argument, we don't care about the padding in the middle - the type definition has all we need to understand which bits must be checked for definedness, and we'd like the attribute to reflect that - i.e. the property of the IR value itself and not its storage representation.

(if it's coerced to i64, then we care).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82316