mehdi_amini added a comment. In https://reviews.llvm.org/D28404#638299, @probinson wrote:
> In https://reviews.llvm.org/D28404#638221, @mehdi_amini wrote: > > > In https://reviews.llvm.org/D28404#638217, @probinson wrote: > > > > > The patch as-is obviously has a massive testing cost, and it's easy to > > > imagine people being tripped up by this in the future. > > > > > > Can you clarify what massive testing cost you're referring to? > > > Well, you just had to modify around 50 tests, and I'd expect some future > tests to have to deal with it too. Maybe "massive" is overstating it but it > seemed like an unusually large number. There are two things: - tests are modified: when adding a new option, it does not seems unusual to me - what impact on future testing. I still don't see any of this future "testing cost" you're referring to right now. > I don't know that just slapping the option on all these tests is really the > most appropriate fix, either, in some cases. I'll look at it more. For instance the ARM test are relying on piping the output of clang to mem2reg to clean the IR and have simpler check patterns (I assume). This is not compatible with optnone obviously. On the other hand I don't want to update the check lines for > 20000 lines in testsclang/test/CodeGen/aarch64-neon-intrinsics.c just to save passing an option. It's likely that some of these test could have their check line adapted, but I didn't see much interest in doing this. https://reviews.llvm.org/D28404 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
