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