> On 2015-Apr-27, at 16:16, Eric Christopher <[email protected]> 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 > <[email protected]> wrote: > > > On 2015 Mar 25, at 16:14, Eric Christopher <[email protected]> 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 > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
