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

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 23 07:58:51 PDT 2020


jdoerfert 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.
+
----------------
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`?


================
Comment at: llvm/docs/LangRef.rst:1260
+    members smaller than their size, and scalars which were never initialized.
+
 .. _gc:
----------------
nlopes wrote:
> What happens when this condition is violated? e.g., when one passes a poison value as argument? Do we get UB? (I think so, but it's worth to make it clear)
Yes, this should be one of the only "value attribute" that will cause UB. The others should poison the value on violation and combined with this one you get UB.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82316





More information about the llvm-commits mailing list