r233227 - Reapply r232888 after applying a fix for -msse4 code generation.
Eric Christopher
echristo at gmail.com
Mon Apr 27 17:14:11 PDT 2015
On Mon, Apr 27, 2015 at 5:13 PM Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:
> > 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!
>
>
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 <
> 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
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150428/78094932/attachment.html>
More information about the cfe-commits
mailing list