probinson added a comment. In https://reviews.llvm.org/D28404#638350, @mehdi_amini wrote:
> 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 50 seems rather more than usual, but whatever. Granted it's not hundreds. > - what impact on future testing. I still don't see any of this future > "testing cost" you're referring to right now. Maybe I worry too much. I am getting a slightly different set of test failures than you did though. I get these failures: CodeGen/aarch64-neon-extract.c CodeGen/aarch64-poly128.c CodeGen/arm-neon-shifts.c CodeGen/arm64-crc32.c And I don't get these failures: CodeGenCXX/apple-kext-indirect-virtual-dtor-call.cpp CodeGenCXX/apple-kext-no-staticinit-section.cpp CodeGenCXX/debug-info-global-ctor-dtor.cpp ================ Comment at: clang/lib/CodeGen/CodeGenModule.cpp:900 + // OptimizeNone implies noinline; we should not be inlining such + // functions. B.addAttribute(llvm::Attribute::NoInline); ---------------- I'd set ShouldAddOptNone = false here, as it's already explicit. ================ Comment at: clang/test/CodeGen/aarch64-neon-2velem.c:1 -// RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon -emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s +// RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon -disable-O0-optnone -disable-O0-optnone -emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s ---------------- Option specified twice. https://reviews.llvm.org/D28404 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
