jdoerfert 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='+')
----------------
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.
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
