[PATCH] D81678: Introduce frozen attribute at call sites for stricter poison analysis

Sun Jun 21 02:05:26 PDT 2020

aqjune added a comment.

In D81678#2105077 <https://reviews.llvm.org/D81678#2105077>, @eugenis wrote:

> > Could you explicitly state that if it is aggregate or vector type all elements and paddings should be frozen too?
>
> If an aggregate is passed as an aggregate at IR level, we should not care about the padding.
>  Unless it's coerced to an integer.

Oh sorry, I missed the comment of @guiand above.
I wanted to suggest making things simpler by having its memory representation always have well-defined bits if stored into memory.
But, if it is helpful for MSan's performance because it is well-defined in C, I think it is fine to not care about the paddings.
I suggest explicitly mentioning that we're not considering paddings of aggregates in the LangRef (at the definition of frozen value too if you want; actually I thought it was already described in the definition, but it didn't).

In D81678#2105429 <https://reviews.llvm.org/D81678#2105429>, @jdoerfert wrote:

> I'm unsure if we want the name `frozen` as it is less helpful to anyone now familiar with the frozen instruction.
>  In a different thread we concluded that we need some sort of `nopoison` as an attribute to convey the behavior is UB if the value would be poison.
>  I would very much prefer a self explanatory spelling here, especially since `nopoison` will be derived from sources other than the frozen instruction.

In the nonnull thread I was in favor of `nopoison` as well, but became to think that `nopoison` is a bit misleading in this patch because it doesn't talk about undef bit.
Personally I preferred `frozen` because whenever there is an ambiguity in its semantics, the precise definition can be made by simply updating the notion of a frozen value, but if a more intuitive spelling is consideration I'm open to any suggestions.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678