Disallow fbasic-block-sections on non-ELF, non-x86 targets.

[snehasish]

Summary

Basic block sections is untested on other platforms and binary formats apart from x86,elf. This patch silently drops the flag if the platform and binary format is not compatible.

[rahmanl]

clang/test/Driver/fbasic-block-sections.c

12

nit, ignore if you want:
CHECK-NOOPT-NOT sounds like double negative? How about using CHECK-OPT-NONX86 or CHECK-OPT-ARM for the check prefix?

[snehasish]

Specify triple for driver tests, address comments.

[MaskRay]

clang/test/Driver/fbasic-block-sections.c

1–6

Consider dropping -linux-gnu because they are insignificant.

The code applies to generic ELF.

[MaskRay]

clang/test/Driver/fbasic-block-sections.c

12

CHECK-TRIPLE should check the diagnostic instead.

[MaskRay]

clang/lib/Driver/ToolChains/Clang.cpp

4892

This should be an error

clang/test/Driver/fbasic-block-sections.c

5

%clang -### -> not %clang -fsyntax-only

[snehasish]

Update test based on review comments.

• Use diag::err instead of warn.
• Drop linux-gnu from the triple for the default test.
• Check for the presence of the diagnostic message.

[snehasish]

Thanks for the quick review @MaskRay, PTAL.

clang/lib/Driver/ToolChains/Clang.cpp

4892

Sure, can you provide a reference/guidance for when err vs warn is appropriate for driver checks? Most of the code in this file uses err but there are some cases of warn being used.

clang/test/Driver/fbasic-block-sections.c

5

What I really want to check for is the absence of the flag for the particular triple. Using -fsyntax-only means that the cc1 args are not emitted. I've added a check for the diagnostic message but retained -### so that I can keep the check for the absence of -fbasic-block-sections=all.

[MaskRay]

clang/lib/Driver/ToolChains/Clang.cpp

4892

I think a warning is suitable for some GCC specific optimization options where a clang port either does not make sense or is not very necessary. This only applies to very common options which ease porting to clang. For clang specific optimizations, especially these with complex setup, I think an error is more useful.

clang/test/Driver/fbasic-block-sections.c

5

Then you can switch to -c

The idea of not %clang is that it makes things clear that it is an error.

12

The NOT pattern is not useful when testing an error.

[snehasish]

Update test to use not tool and -c flag.

• Use the not tool to indicate failure is expected.
• Use -c instead of -### for failing invocations.

[snehasish]

Thanks for explaining the rationale, PTAL.

[MaskRay]

Looks great!

clang/lib/Driver/ToolChains/Clang.cpp

#endif
   bool UseSeparateSections = isUseSeparateSections(Triple);
 
   if (Args.hasFlag(options::OPT_ffunction_sections,
                    options::OPT_fno_function_sections, UseSeparateSections)) {
     CmdArgs.push_back("-ffunction-sections");
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_fbasic_block_sections_EQ)) {
+    if (Triple.isX86() && Triple.isOSBinFormatELF()) {
       StringRef Val = A->getValue();
       if (Val != "all" && Val != "labels" && Val != "none" &&
           !(Val.startswith("list=") && llvm::sys::fs::exists(Val.substr(5))))
         D.Diag(diag::err_drv_invalid_value)
             << A->getAsString(Args) << A->getValue();
       else
         A->render(Args, CmdArgs);
+    } else {
+      D.Diag(diag::err_drv_unsupported_opt_for_target)
+          << A->getAsString(Args) << TripleStr;
+    }
   }
 
   if (Args.hasFlag(options::OPT_fdata_sections, options::OPT_fno_data_sections,
                    UseSeparateSections)) {
     CmdArgs.push_back("-fdata-sections");
   }
 
   if (!Args.hasFlag(options::OPT_funique_section_names,

clang/test/Driver/fbasic-block-sections.c

-// RUN: %clang -### -fbasic-block-sections=none %s -S 2>&1 | FileCheck -check-prefix=CHECK-OPT-NONE %s
-// RUN: %clang -### -fbasic-block-sections=all %s -S 2>&1 | FileCheck -check-prefix=CHECK-OPT-ALL %s
-// RUN: %clang -### -fbasic-block-sections=list=%s %s -S 2>&1 | FileCheck -check-prefix=CHECK-OPT-LIST %s
-// RUN: %clang -### -fbasic-block-sections=labels %s -S 2>&1 | FileCheck -check-prefix=CHECK-OPT-LABELS %s
+// RUN: %clang -### -target x86_64 -fbasic-block-sections=none %s -S 2>&1 | FileCheck -check-prefix=CHECK-OPT-NONE %s
+// RUN: %clang -### -target x86_64 -fbasic-block-sections=all %s -S 2>&1 | FileCheck -check-prefix=CHECK-OPT-ALL %s
+// RUN: %clang -### -target x86_64 -fbasic-block-sections=list=%s %s -S 2>&1 | FileCheck -check-prefix=CHECK-OPT-LIST %s
+// RUN: %clang -### -target x86_64 -fbasic-block-sections=labels %s -S 2>&1 | FileCheck -check-prefix=CHECK-OPT-LABELS %s
+// RUN: not %clang -c -target arm-unknown-linux -fbasic-block-sections=all %s -S 2>&1 | FileCheck -check-prefix=CHECK-TRIPLE %s
+// RUN: not %clang -c -target x86_64-apple-darwin10 -fbasic-block-sections=all %s -S 2>&1 | FileCheck -check-prefix=CHECK-TRIPLE %s
 //
-// CHECK-OPT-NONE: "-fbasic-block-sections=none"
-// CHECK-OPT-ALL: "-fbasic-block-sections=all"
-// CHECK-OPT-LIST: "-fbasic-block-sections={{[^ ]*}}fbasic-block-sections.c"
+// CHECK-OPT-NONE:   "-fbasic-block-sections=none"
+// CHECK-OPT-ALL:    "-fbasic-block-sections=all"
+// CHECK-OPT-LIST:   "-fbasic-block-sections={{[^ ]*}}fbasic-block-sections.c"
 // CHECK-OPT-LABELS: "-fbasic-block-sections=labels"
+// CHECK-TRIPLE:     error: unsupported option '-fbasic-block-sections=all' for target