[PATCH] D83004: [UpdateCCTestChecks] Include generated functions if asked
David Greene via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 7 08:02:00 PDT 2020
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:
> > 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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83004/new/
https://reviews.llvm.org/D83004
More information about the cfe-commits
mailing list