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

Sat Jun 20 13:14:27 PDT 2020

jdoerfert requested changes to this revision.
jdoerfert added a comment.
This revision now requires changes to proceed.

First, apologies for being late, I didn't properly monitor the list recently.

---

This diff is impossible to review and later to understand.

I would ask you to split it, at least for review purposes, such that (1) lang ref changes, (2) code changes, (3) test changes are separate.
Please also upload it with a full diff (at least for (1) and (2)).

The revision title needs to be updated (maybe add `[LangRef]` or `[Attr]`), and the commit message needs more explanation of the reasons and semantics.

---

Lang Ref (https://reviews.llvm.org/differential/changeset/?ref=2023449 <- so people can find it)

I think the spelling is not in line with other attributes and needs to be changed.

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 lang ref this is listed with and shown as string attribute. It should be with the regular ones (as it is implemented as such). It is also not target specific.

---

All in all we need to open this up to more people and go over the lang ref changes again.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678