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

Tue Jun 23 12:54:06 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.
+
----------------
jdoerfert wrote:
> eugenis wrote:
> > 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).
> > 
> > 
> Does this mean we still need more than `nopoison`?
Sorry, I don't understand - are saying the attribute can be specified in terms of poison values and not undef? I don't think it would work, we need it to cover something that's loaded from an uninitialized memory location and passed to a function call as is, and those are undef, not poison.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82316