tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

LGTM with the test change suggested below.



================
Comment at: clang/test/CodeGenCXX/virtual-function-elimination.cpp:2
 // RUN: %clang_cc1 -triple x86_64-unknown-linux -flto -flto-unit 
-fvirtual-function-elimination -fwhole-program-vtables -emit-llvm -o - %s | 
FileCheck %s
-
+// RUN: %clang -target x86_64-unknown-linux -flto 
-fno-virtual-function-elimination -fwhole-program-vtables -S -emit-llvm -o - %s 
| FileCheck %s -check-prefix=NOVFE
 
----------------
Since VFE isn't the default, I suspect you need to put 
-fvirtual-function-elimination before the -fno-virtual-function-elimination for 
it to really test that it is disabling VFE. (Although I realize that this does 
test this patch fix since presumably it complained about the unknown option 
before fixing it, but still it would be good to ensure that we're also testing 
that the -fno- form has the desired affect.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88349/new/

https://reviews.llvm.org/D88349

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to