[PATCH] D83004: [UpdateCCTestChecks] Include generated functions if asked

[David Greene via Phabricator via llvm-commits]

greened marked an inline comment as done.
greened added inline comments.


================
Comment at: llvm/utils/update_cc_test_checks.py:133
+  parser.add_argument('--include-generated-funcs', action='store_true',
+                      help='Output checks for functions not in source')
   parser.add_argument('tests', nargs='+')
----------------
jdoerfert wrote:
> greened wrote:
> > jdoerfert wrote:
> > > greened wrote:
> > > > greened wrote:
> > > > > jdoerfert wrote:
> > > > > > I think this should go into common.py (after D78618). I would also make this the default but OK.
> > > > > Yes I suppose it should in case `opt` and friends generate functions.  I hadn't considered that use-case.
> > > > > 
> > > > > While I would like to make it default unfortunately it would require updating a bunch of the existing clang tests which doesn't seem too friendly.  See the patch update comment for details.
> > > > > 
> > > > Just realized it wouldn't necessarily require regeneration of tests, it would just cause regenerated tests to change a lot when they are eventually regenerated.  We should discuss as to whether that's acceptable.  I think for now this should be non-default to at least get the functionality in without disturbing existing users and then we can discuss a separate change to make it default.
> > > > 
> > > > It's also possible we could change how clang orders functions.  I discovered there's a difference in clang 10 vs. 11 in the order functions are output when OpenMP outlining happens.  clang 10 seems to preserve the source order of functions and clang 11 does not.  Perhaps that needs to be fixed as I don't know whether that change was intentional or not.
> > > Best case, without the option the original behavior is preserved. Is that not the case?
> > That's right.  I was referring to making this behavior default.  If we do that, we could clean up the script code a bit but it would mean clang tests would change pretty dramatically when they are regenerated.  If we fix the clang output, the test changes wouldn't be so dramatic.
> > 
> > The way clang is behaving now, I would expect any tests that use `-fopenmp`, have multiple functions with OpenMP regions and use function prototypes for some of those functions would break given clang's reordering of function definitions.  Perhaps we don't have any tests like that though.
> We (almost) do not have OpenMP tests with autogenerated test lines. Partially, because we do not test new functions. I would really like this to be available for OpenMP, both in _cc_ and IR tests. If people can opt out of this, especially if the default is "off", the ordering is not a problem (IMHO). With UTC_ARGS we also remember the choice so I really don't see the downside to this being in the common part for all scripts.
I agree it can be in the common part. I'll move it there and will leave it off by default so it doesn't disturb existing tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83004