[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

Evgenii Stepanov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 6 14:31:09 PDT 2020


eugenis added a comment.

In D82317#2200789 <https://reviews.llvm.org/D82317#2200789>, @rjmccall wrote:

> Are you seriously adding an attribute to literally every argument and return value?  Why is this the right representation?

We've discussed quite a few options in D81678 <https://reviews.llvm.org/D81678>, which is a better place for this conversation.

The original patch used an inverted attribute, partialinit, which would be a lot less common. The problem with that is that it redefines the meaning of when an argument does not have the attribute, which breaks compatibility with existing bitcode and requires updating all frontends and any code that emits new functions or edits function signatures in IR.

There was an option of a function-level attribute in addition to this one, which would mean "all arguments and the return value are noundef". That would be a bit uncomfortable to work with in IR transforms, and, arguably, would not reduce the total number of attributes that much.

There are also a bunch of options that introduce divergence between

- tests and real use - a test-only cc1 flag to suppress attribute generation
- msan and non-msan compilation - but the reviewers in D81678 <https://reviews.llvm.org/D81678> seem pretty excited to use this info for undef/poison optimizations in all configurations

In fact, we already have the test-only cc1 flag that disables the new attribute, and use it in some of the tests. This could be extended to all tests, that should make the tests diff smaller and more mechanical,  and hopefully help with the downstream concerns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82317



More information about the cfe-commits mailing list