arsenm requested changes to this revision. arsenm added a comment. This revision now requires changes to proceed.
In D140467#4010378 <https://reviews.llvm.org/D140467#4010378>, @pengfei wrote: > Use FastMathFlagGuard instead, thanks @foad! > > In D140467#4010296 <https://reviews.llvm.org/D140467#4010296>, @arsenm wrote: > >> Needs tests. I couldn’t find any for the base builtins either > > I don't think so. We have tested their intrinsic wrappers, e.g., > https://github.com/llvm/llvm-project/blob/main/clang/test/CodeGen/X86/avx512fp16-builtins.c#L4451 These are not testing use of these builtins correctly > We don't need to test those target specific builtins. They are neither > guaranteed to be cross compiler compatible nor release by release. If it exists it must be tested. > Besides, we have thousands of intrinsics in X86. Adding their builtin tests > have no any benefits and increase the time in lit tests. Every piece of code generation needs to be tested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140467/new/ https://reviews.llvm.org/D140467 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
