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 

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 


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

  rG LLVM Github Monorepo



cfe-commits mailing list

Reply via email to