r233227 - Reapply r232888 after applying a fix for -msse4 code generation.

[Duncan P. N. Exon Smith]

> On 2015 Mar 25, at 16:14, Eric Christopher <echristo at gmail.com> wrote:
> 
> Author: echristo
> Date: Wed Mar 25 18:14:47 2015
> New Revision: 233227
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=233227&view=rev
> Log:
> Reapply r232888

This is awesome, btw :).

> after applying a fix for -msse4 code generation.
> 
> As a note, any target that uses fake target features via command
> line options will have similar problems.
> 
> Added:
>    cfe/trunk/test/CodeGen/function-target-features.c
> Modified:
>    cfe/trunk/lib/CodeGen/CGCall.cpp
> 
> Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=233227&r1=233226&r2=233227&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Mar 25 18:14:47 2015
> @@ -31,6 +31,7 @@
> #include "llvm/IR/InlineAsm.h"
> #include "llvm/IR/Intrinsics.h"
> #include "llvm/Transforms/Utils/Local.h"
> +#include <sstream>
> using namespace clang;
> using namespace CodeGen;
> 
> @@ -1475,6 +1476,26 @@ void CodeGenModule::ConstructAttributeLi
> 
>     if (!CodeGenOpts.StackRealignment)
>       FuncAttrs.addAttribute("no-realign-stack");
> +
> +    // Add target-cpu and target-features work if they differ from the defaults.
> +    std::string &CPU = getTarget().getTargetOpts().CPU;
> +    if (CPU != "" && CPU != getTarget().getTriple().getArchName())

Why not the following?

    if (CPU != "")

The frontend and backend don't necessarily have the same default CPU.
IMO that's a bug that should be fixed eventually, but I think we
should encode the CPU to ensure that it matches up on the other side.

> +      FuncAttrs.addAttribute("target-cpu", getTarget().getTargetOpts().CPU);

You can reuse the `CPU` local here.

> +
> +    // TODO: FeaturesAsWritten gets us the features on the command line,
> +    // for canonicalization purposes we might want to avoid putting features
> +    // in the target-features set if we know it'll be one of the default
> +    // features in the backend, e.g. corei7-avx and +avx.

Same point as above for target-cpu.  I don't think we should rely on
defaults matching in the backend until the defaults come from the same
codebase.

> +    std::vector<std::string> &Features =
> +        getTarget().getTargetOpts().FeaturesAsWritten;
> +    if (!Features.empty()) {

Even here, shouldn't we just encode `"target-features"=""` when
Features is empty?

> +      std::stringstream S;
> +      std::copy(Features.begin(), Features.end(),
> +                std::ostream_iterator<std::string>(S, ","));
> +      // The drop_back gets rid of the trailing space.
> +      FuncAttrs.addAttribute("target-features",
> +                             StringRef(S.str()).drop_back(1));
> +    }
>   }
> 
>   ClangToLLVMArgMapping IRFunctionArgs(getContext(), FI);
> 
> Added: cfe/trunk/test/CodeGen/function-target-features.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/function-target-features.c?rev=233227&view=auto
> ==============================================================================
> --- cfe/trunk/test/CodeGen/function-target-features.c (added)
> +++ cfe/trunk/test/CodeGen/function-target-features.c Wed Mar 25 18:14:47 2015
> @@ -0,0 +1,21 @@
> +// This test verifies that we produce target-cpu and target-features attributes
> +// on functions when they're different from the standard cpu and have written
> +// features.
> +
> +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -target-feature +avx | FileCheck %s -check-prefix=AVX-FEATURE
> +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -target-feature +avx | FileCheck %s -check-prefix=AVX-NO-CPU
> +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -target-feature +avx512f -target-feature +avx512er | FileCheck %s -check-prefix=TWO-AVX
> +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -target-cpu corei7 | FileCheck %s -check-prefix=CORE-CPU
> +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -target-cpu corei7 -target-feature +avx | FileCheck %s -check-prefix=CORE-CPU-AND-FEATURES
> +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -target-cpu x86-64 | FileCheck %s -check-prefix=X86-64-CPU-NOT
> +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -target-cpu corei7-avx -target-feature -avx | FileCheck %s -check-prefix=AVX-MINUS-FEATURE
> +
> +void foo() {}
> +
> +// AVX-FEATURE: "target-features"="+avx"
> +// AVX-NO-CPU-NOT: target-cpu
> +// TWO-AVX: "target-features"="+avx512f,+avx512er"
> +// CORE-CPU: "target-cpu"="corei7"
> +// CORE-CPU-AND-FEATURES: "target-cpu"="corei7" "target-features"="+avx"
> +// X86-64-CPU-NOT: "target-cpu"
> +// AVX-MINUS-FEATURE: "target-features"="-avx"
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits