On Mon, Apr 27, 2015 at 5:13 PM Duncan P. N. Exon Smith < [email protected]> wrote:
> > 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! > > Yep. Thanks for the review! -eric > > > > 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
