tra accepted this revision.
tra added a subscriber: echristo.
tra added a comment.
This revision is now accepted and ready to land.
@echristo : FYI, just in case you have an opinion on bringing required feature
constraint checks one step closer to being Turing-complete. :-)
LGTM as far as syntax and use for NVPTX goes. No opinion on whether it
**should** go into codegen.
While the change appears to be sensible on general principles, I'm not aware of
any specific case where it's actually needed.
It would help if you could elaborate on why you need this functionality.
================
Comment at: clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp:16
+ };
+ ASSERT_TRUE(doCheck("A|B,C|D", "A"));
+ ASSERT_FALSE(doCheck("(A|B),(C|D)", "A"));
----------------
LiuChen3 wrote:
> pengfei wrote:
> > tra wrote:
> > > Is `doCheck("A,B,C,D", ...)` expected to work? I'd add test cases for
> > > that, too.
> > >
> > >
> > I may not get your point here.
> > We added the unittest to check the correctness for the function. It's an
> > easier and more flexible way checking for complex functions than using llc.
> > I don't know if there's criteria for the unittests.
> I think @tra 's mean is adding tests like ASSERT_FALSE(doCheck("A,B,C,D",
> "A")); .
Yes. I should've phrased it better. I meant tests w/o parens in the feature
requirements.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89184/new/
https://reviews.llvm.org/D89184
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
