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

[Duncan P. N. Exon Smith]

> On 2015-Apr-27, at 16:16, Eric Christopher <echristo at gmail.com> wrote:
> 
> Hi Duncan,
> 
> Sorry for the delay, I took a while to think about this in context and agree that it's probably the best way to deal with this. Have the front end be very explicit in what it's giving to the back end rather than try to be concise. FWIW until the patch about migrating target attributes to constructors/destructors this wouldn't have worked - there are a few other places I think are missing, but for now:

We'll catch the others eventually I'm sure.  Thanks for this!

> 
> dzur:~/sources/llvm/tools/clang> git svn dcommit 
> Committing to https://llvm.org/svn/llvm-project/cfe/trunk ...
> 	M	lib/CodeGen/CGCall.cpp
> 	M	test/CodeGen/function-target-features.c
> Committed r235936
> 
> -eric
> 
> On Tue, Mar 31, 2015 at 8:59 PM Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
> > 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
>