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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
