nlopes added a comment.
I'm a bit concerned with this patch as it increases the amount of UB that LLVM
exploits without any study of the impact.
For example, right now it's ok do this with clang (not with constants; make it
less trivial so clang doesn't fold it right away):
int f() { return INT_MAX + 1; }
While technically this is UB in C, when lowered to LLVM IR, this function
returns poison.
When the frozen attribute is attached, the function will now trigger UB in LLVM
IR as well.
Is this what we want? It would be worthwhile to at least compile a few programs
to check if they break. Also, what's the plan to detect these cases in ubsan?
We shouldn't be exploiting new UB without having good warnings/checks in place
IMO.
Note that pure function calls with 'frozen' arguments become harder to hoist
from loops, for example. Since now calling this pure function can trigger UB if
one of the arguments is poison. You would need to introduce frozen beforehand.
I don't see this issue addressed in this patch.
For msan & friends, this patch makes sense, as they operate in a rely-guarantee
way. Initialization of memory is checked, so it can be relied upon by the
compiler later on.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81678/new/
https://reviews.llvm.org/D81678
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
