Move the emission of strict enum range metadata behind a flag (the same

flag as GCC uses: -fstrict-enums). There is a *lot* of code making
unwarranted assumptions about the underlying type of enums, and it
doesn't seem entirely reasonable to eagerly break all of it.

Much more importantly, the current state of affairs is *very* good at
optimizing based upon this information, which causes failures that are
very distant from the actual enum. Before we push for enabling this by
default, I think we need to implement -fcatch-undefined-behavior support
for instrumenting and trapping whenever we store or load a value outside
of the range. That way we can track down the misbehaving code very
quickly.

I discussed this with Rafael, and currently the only important cases he
is aware of are the bool range-based optimizations which are staying
hard enabled. We've not seen any issue with those either, and they are
much more important for performance.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@153550 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Driver/CC1Options.td b/include/clang/Driver/CC1Options.td
index 46c4ed89b5c62c7acafc3266028039f2e798d076..956f2a65cefd43abd61948875b0f2fc1180c8adc 100644 (file)
--- a/include/clang/Driver/CC1Options.td
+++ b/include/clang/Driver/CC1Options.td
@@ -178,6 +178,9 @@ def ffunction_sections : Flag<"-ffunction-sections">,
   HelpText<"Place each function in its own section (ELF Only)">;
 def fdata_sections : Flag<"-fdata-sections">,
   HelpText<"Place each data in its own section (ELF Only)">;
+def fstrict_enums : Flag<"-fstrict-enums">,
+  HelpText<"Enable optimizations based on the strict definition of an enum's "
+           "value range.">;
 def ftrap_function_EQ : Joined<"-ftrap-function=">,
   HelpText<"Issue call to specified function rather than a trap instruction">;
 def funroll_loops : Flag<"-funroll-loops">,

diff --git a/include/clang/Driver/Options.td b/include/clang/Driver/Options.td
index eab8e8f331140a282a6b7eedee0915ee0e90f874..6bc0a7d0b528e81020305586de17ecc86bdf0ca5 100644 (file)
--- a/include/clang/Driver/Options.td
+++ b/include/clang/Driver/Options.td
@@ -444,6 +444,7 @@ def fno_show_source_location : Flag<"-fno-show-source-location">, Group<f_Group>
 def fno_spell_checking : Flag<"-fno-spell-checking">, Group<f_Group>;
 def fno_stack_protector : Flag<"-fno-stack-protector">, Group<f_Group>;
 def fno_strict_aliasing : Flag<"-fno-strict-aliasing">, Group<f_Group>;
+def fno_strict_enums : Flag<"-fno-strict-enums">, Group<f_Group>;
 def fno_strict_overflow : Flag<"-fno-strict-overflow">, Group<f_Group>;
 def fno_threadsafe_statics : Flag<"-fno-threadsafe-statics">, Group<f_Group>;
 def fno_use_cxa_atexit : Flag<"-fno-use-cxa-atexit">, Group<f_Group>;
@@ -515,6 +516,7 @@ def fsigned_char : Flag<"-fsigned-char">, Group<f_Group>;
 def fstack_protector_all : Flag<"-fstack-protector-all">, Group<f_Group>;
 def fstack_protector : Flag<"-fstack-protector">, Group<f_Group>;
 def fstrict_aliasing : Flag<"-fstrict-aliasing">, Group<f_Group>;
+def fstrict_enums : Flag<"-fstrict-enums">, Group<f_Group>;
 def fstrict_overflow : Flag<"-fstrict-overflow">, Group<f_Group>;
 def fsyntax_only : Flag<"-fsyntax-only">, Flags<[DriverOption]>;
 def ftabstop_EQ : Joined<"-ftabstop=">, Group<f_Group>;

diff --git a/include/clang/Frontend/CodeGenOptions.h b/include/clang/Frontend/CodeGenOptions.h
index 59509331a0407897b832b3a07239b3a63a25f0ce..e844f8869c40a6720bdb279254a4c9139394f836 100644 (file)
--- a/include/clang/Frontend/CodeGenOptions.h
+++ b/include/clang/Frontend/CodeGenOptions.h
@@ -94,6 +94,7 @@ public:
   unsigned SaveTempLabels    : 1; /// Save temporary labels.
   unsigned SimplifyLibCalls  : 1; /// Set when -fbuiltin is enabled.
   unsigned SoftFloat         : 1; /// -soft-float.
+  unsigned StrictEnums       : 1; /// Optimize based on strict enum definition.
   unsigned TimePasses        : 1; /// Set when -ftime-report is enabled.
   unsigned UnitAtATime       : 1; /// Unused. For mirroring GCC optimization
                                   /// selection.
@@ -205,6 +206,7 @@ public:
     SaveTempLabels = 0;
     SimplifyLibCalls = 1;
     SoftFloat = 0;
+    StrictEnums = 0;
     TimePasses = 0;
     UnitAtATime = 1;
     UnrollLoops = 0;

diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp
index a9f87e67d4fa0accfc3ed460e6bf0e046bb82999..511914884f68bbc620a5275ae2fecdc133732129 100644 (file)
--- a/lib/CodeGen/CGExpr.cpp
+++ b/lib/CodeGen/CGExpr.cpp
@@ -872,8 +872,9 @@ static bool hasBooleanRepresentation(QualType Ty) {
 
 llvm::MDNode *CodeGenFunction::getRangeForLoadFromType(QualType Ty) {
   const EnumType *ET = Ty->getAs<EnumType>();
-  bool IsRegularCPlusPlusEnum = getLangOpts().CPlusPlus && ET &&
-    !ET->getDecl()->isFixed();
+  bool IsRegularCPlusPlusEnum = (getLangOpts().CPlusPlus && ET &&
+                                 CGM.getCodeGenOpts().StrictEnums &&
+                                 !ET->getDecl()->isFixed());
   bool IsBool = hasBooleanRepresentation(Ty);
   llvm::Type *LTy;
   if (!IsBool && !IsRegularCPlusPlusEnum)

diff --git a/lib/Driver/Tools.cpp b/lib/Driver/Tools.cpp
index 52c22b13a4a1e2e92cb74e19fef90efec68acfa1..ff034d685ded66d5843b13796e57f15f8a2a9443 100644 (file)
--- a/lib/Driver/Tools.cpp
+++ b/lib/Driver/Tools.cpp
@@ -1487,6 +1487,9 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
                     options::OPT_fno_strict_aliasing,
                     getToolChain().IsStrictAliasingDefault()))
     CmdArgs.push_back("-relaxed-aliasing");
+  if (Args.hasFlag(options::OPT_fstrict_enums, options::OPT_fno_strict_enums,
+                   false))
+    CmdArgs.push_back("-fstrict-enums");
   if (!Args.hasFlag(options::OPT_foptimize_sibling_calls,
                     options::OPT_fno_optimize_sibling_calls))
     CmdArgs.push_back("-mdisable-tail-calls");

diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp
index 0f68591c963cd635443e67e17564de89731a0800..48b8049cd094efc3105c110130f842f377e02457 100644 (file)
--- a/lib/Frontend/CompilerInvocation.cpp
+++ b/lib/Frontend/CompilerInvocation.cpp
@@ -286,6 +286,8 @@ static void CodeGenOptsToArgs(const CodeGenOptions &Opts, ToArgsList &Res) {
     Res.push_back("-fno-dwarf-directory-asm");
   if (Opts.SoftFloat)
     Res.push_back("-msoft-float");
+  if (Opts.StrictEnums)
+    Res.push_back("-fstrict-enums");
   if (Opts.UnwindTables)
     Res.push_back("-munwind-tables");
   if (Opts.RelocationModel != "pic")
@@ -1135,6 +1137,7 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
   Opts.NoDwarf2CFIAsm = Args.hasArg(OPT_fno_dwarf2_cfi_asm);
   Opts.NoDwarfDirectoryAsm = Args.hasArg(OPT_fno_dwarf_directory_asm);
   Opts.SoftFloat = Args.hasArg(OPT_msoft_float);
+  Opts.StrictEnums = Args.hasArg(OPT_fstrict_enums);
   Opts.UnsafeFPMath = Args.hasArg(OPT_menable_unsafe_fp_math) ||
                       Args.hasArg(OPT_cl_unsafe_math_optimizations) ||
                       Args.hasArg(OPT_cl_fast_relaxed_math);

diff --git a/test/CodeGenCXX/pr12251.cpp b/test/CodeGenCXX/pr12251.cpp
index 310e60d3e5203a9e04c766e0a9fe9cac95e354ce..a9920c073388dc9f87916f15ca14b58d133a0b0f 100644 (file)
--- a/test/CodeGenCXX/pr12251.cpp
+++ b/test/CodeGenCXX/pr12251.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 %s -emit-llvm -O1 -relaxed-aliasing -std=c++11 -o - | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -O1 -relaxed-aliasing -fstrict-enums -std=c++11 -o - | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -O1 -relaxed-aliasing -std=c++11 -o - | FileCheck --check-prefix=NO-STRICT-ENUMS %s
 
 bool f(bool *x) {
   return *x;
@@ -6,6 +7,12 @@ bool f(bool *x) {
 // CHECK: define zeroext i1 @_Z1fPb
 // CHECK: load i8* %{{.*}}, align 1, !range !0
 
+// Only enum-tests follow. Ensure that after the bool test, no further range
+// metadata shows up when strict enums are disabled.
+// NO-STRICT-ENUMS: define zeroext i1 @_Z1fPb
+// NO-STRICT-ENUMS: load i8* %{{.*}}, align 1, !range !0
+// NO-STRICT-ENUMS-NOT: !range
+
 enum e1 { };
 e1 g1(e1 *x) {
   return *x;