[PATCH] D28404: IRGen: Add optnone attribute on function during O0

Paul Robinson via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 6 15:36:00 PST 2017


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





More information about the cfe-commits mailing list