[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-commits
Subject: Re: r246229 - [X86] Conditionalize Darwin MaxVectorAlign on the presence of AVX.
From: Ahmed Bougacha via cfe-commits <cfe-commits () lists ! llvm ! org>
Date: 2015-08-31 23:03:19
Message-ID: CAOprbYRvFhyRyRUvvnEBHk8JF2N9cMoRYOZBfq_Ex25nSZg+3g () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On Thu, Aug 27, 2015 at 7:19 PM, Eric Christopher <echristo@gmail.com>
wrote:
> Hi Ahmed,
>
> A quick note: I think this is going to fail in the presence of the target
> attribute. I.e. if someone decorates a function with
> __attribute__((target("avx512"))) this is unlikely to catch that.
>
You're right; any ideas on how to handle this? In particular, has any of
IRGen (except attributes of course) needed to know about it until know?
IIRC it's directly lowered to IR function attributes, so this sounds like a
big change. Worst case we can restore the unconditional max alignment?
> Also, should we do this for all of the x86 OSes?
>
I'm not sure: MaxVectorAlign was only set for the Darwin targets, but I
think it would make sense everywhere.
Let's see what people say: http://reviews.llvm.org/D12505
Thanks for the comments!
-Ahmed
> -eric
>
> On Thu, Aug 27, 2015 at 3:31 PM Ahmed Bougacha via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> > Author: ab
> > Date: Thu Aug 27 17:30:38 2015
> > New Revision: 246229
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=246229&view=rev
> > Log:
> > [X86] Conditionalize Darwin MaxVectorAlign on the presence of AVX.
> >
> > There's no point in using a larger alignment if we have no instructions
> > that would benefit from it.
> >
> > Differential Revision: http://reviews.llvm.org/D12389
> >
> > Modified:
> > cfe/trunk/lib/Basic/Targets.cpp
> > cfe/trunk/test/CodeGen/vector-alignment.c
> >
> > Modified: cfe/trunk/lib/Basic/Targets.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=246229&r1=246228&r2=246229&view=diff
> >
> > ==============================================================================
> > --- cfe/trunk/lib/Basic/Targets.cpp (original)
> > +++ cfe/trunk/lib/Basic/Targets.cpp Thu Aug 27 17:30:38 2015
> > @@ -3629,13 +3629,21 @@ public:
> > LongDoubleWidth = 128;
> > LongDoubleAlign = 128;
> > SuitableAlign = 128;
> > - MaxVectorAlign = 256;
> > SizeType = UnsignedLong;
> > IntPtrType = SignedLong;
> > DataLayoutString = "e-m:o-p:32:32-f64:32:64-f80:128-n8:16:32-S128";
> > HasAlignMac68kSupport = true;
> > }
> >
> > + bool handleTargetFeatures(std::vector<std::string> &Features,
> > + DiagnosticsEngine &Diags) override {
> > + if
> > (!DarwinTargetInfo<X86_32TargetInfo>::handleTargetFeatures(Features,
> > + Diags))
> > + return false;
> > + // Now that we know if we have AVX, we can decide how to align
> > vectors.
> > + MaxVectorAlign = hasFeature("avx") ? 256 : 128;
> > + return true;
> > + }
> > };
> >
> > // x86-32 Windows target
> > @@ -3986,13 +3994,22 @@ public:
> > DarwinX86_64TargetInfo(const llvm::Triple &Triple)
> > > DarwinTargetInfo<X86_64TargetInfo>(Triple) {
> > Int64Type = SignedLongLong;
> > - MaxVectorAlign = 256;
> > // The 64-bit iOS simulator uses the builtin bool type for
> > Objective-C.
> > llvm::Triple T = llvm::Triple(Triple);
> > if (T.isiOS())
> > UseSignedCharForObjCBool = false;
> > DataLayoutString = "e-m:o-i64:64-f80:128-n8:16:32:64-S128";
> > }
> > +
> > + bool handleTargetFeatures(std::vector<std::string> &Features,
> > + DiagnosticsEngine &Diags) override {
> > + if
> > (!DarwinTargetInfo<X86_64TargetInfo>::handleTargetFeatures(Features,
> > + Diags))
> > + return false;
> > + // Now that we know if we have AVX, we can decide how to align
> > vectors.
> > + MaxVectorAlign = hasFeature("avx") ? 256 : 128;
> > + return true;
> > + }
> > };
> >
> > class OpenBSDX86_64TargetInfo : public
> > OpenBSDTargetInfo<X86_64TargetInfo> {
> >
> > Modified: cfe/trunk/test/CodeGen/vector-alignment.c
> > URL:
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/vector-alignment.c?rev=246229&r1=246228&r2=246229&view=diff
> >
> > ==============================================================================
> > --- cfe/trunk/test/CodeGen/vector-alignment.c (original)
> > +++ cfe/trunk/test/CodeGen/vector-alignment.c Thu Aug 27 17:30:38 2015
> > @@ -1,38 +1,51 @@
> > -// RUN: %clang_cc1 -w -triple x86_64-apple-darwin10 -emit-llvm -o - %s |
> > FileCheck %s
> > +// RUN: %clang_cc1 -w -triple x86_64-apple-darwin10 \
> > +// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=ALL
> > --check-prefix=SSE
> > +// RUN: %clang_cc1 -w -triple i386-apple-darwin10 \
> > +// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=ALL
> > --check-prefix=SSE
> > +// RUN: %clang_cc1 -w -triple x86_64-apple-darwin10 -target-feature +avx
> > \
> > +// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=ALL
> > --check-prefix=AVX
> > +// RUN: %clang_cc1 -w -triple i386-apple-darwin10 -target-feature +avx
> > \
> > +// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=ALL
> > --check-prefix=AVX
> > // rdar://11759609
> >
> > // At or below target max alignment with no aligned attribute should
> > align based
> > // on the size of vector.
> > double __attribute__((vector_size(16))) v1;
> > -// CHECK: @v1 {{.*}}, align 16
> > +// SSE: @v1 {{.*}}, align 16
> > +// AVX: @v1 {{.*}}, align 16
> > double __attribute__((vector_size(32))) v2;
> > -// CHECK: @v2 {{.*}}, align 32
> > +// SSE: @v2 {{.*}}, align 16
> > +// AVX: @v2 {{.*}}, align 32
> >
> > // Alignment above target max alignment with no aligned attribute should
> > align
> > // based on the target max.
> > double __attribute__((vector_size(64))) v3;
> > -// CHECK: @v3 {{.*}}, align 32
> > +// SSE: @v3 {{.*}}, align 16
> > +// AVX: @v3 {{.*}}, align 32
> > double __attribute__((vector_size(1024))) v4;
> > -// CHECK: @v4 {{.*}}, align 32
> > +// SSE: @v4 {{.*}}, align 16
> > +// AVX: @v4 {{.*}}, align 32
> >
> > // Aliged attribute should always override.
> > double __attribute__((vector_size(16), aligned(16))) v5;
> > -// CHECK: @v5 {{.*}}, align 16
> > +// ALL: @v5 {{.*}}, align 16
> > double __attribute__((vector_size(16), aligned(64))) v6;
> > -// CHECK: @v6 {{.*}}, align 64
> > +// ALL: @v6 {{.*}}, align 64
> > double __attribute__((vector_size(32), aligned(16))) v7;
> > -// CHECK: @v7 {{.*}}, align 16
> > +// ALL: @v7 {{.*}}, align 16
> > double __attribute__((vector_size(32), aligned(64))) v8;
> > -// CHECK: @v8 {{.*}}, align 64
> > +// ALL: @v8 {{.*}}, align 64
> >
> > // Check non-power of 2 widths.
> > double __attribute__((vector_size(24))) v9;
> > -// CHECK: @v9 {{.*}}, align 32
> > +// SSE: @v9 {{.*}}, align 16
> > +// AVX: @v9 {{.*}}, align 32
> > double __attribute__((vector_size(40))) v10;
> > -// CHECK: @v10 {{.*}}, align 32
> > +// SSE: @v10 {{.*}}, align 16
> > +// AVX: @v10 {{.*}}, align 32
> >
> > // Check non-power of 2 widths with aligned attribute.
> > double __attribute__((vector_size(24), aligned(64))) v11;
> > -// CHECK: @v11 {{.*}}, align 64
> > +// ALL: @v11 {{.*}}, align 64
> > double __attribute__((vector_size(80), aligned(16))) v12;
> > -// CHECK: @v12 {{.*}}, align 16
> > +// ALL: @v12 {{.*}}, align 16
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
>
[Attachment #5 (text/html)]
<div dir="ltr">On Thu, Aug 27, 2015 at 7:19 PM, Eric Christopher <span \
dir="ltr"><<a href="mailto:echristo@gmail.com" \
target="_blank">echristo@gmail.com</a>></span> wrote:<br><div \
class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" \
style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div \
dir="ltr">Hi Ahmed,<div><br></div><div>A quick note: I think this is going to fail in \
the presence of the target attribute. I.e. if someone decorates a function with \
__attribute__((target("avx512"))) this is unlikely to catch \
that.</div></div></blockquote><div><br></div><div>You're right; any ideas on how \
to handle this? In particular, has any of IRGen (except attributes of course) needed \
to know about it until know? IIRC it's directly lowered to IR function \
attributes, so this sounds like a big change. Worst case we can restore the \
unconditional max alignment?</div><div> </div><blockquote class="gmail_quote" \
style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div \
dir="ltr"><div>Also, should we do this for all of the x86 \
OSes?</div></div></blockquote><div><br></div><div>I'm not sure: MaxVectorAlign \
was only set for the Darwin targets, but I think it would make sense \
everywhere.</div><div>Let's see what people say: <a \
href="http://reviews.llvm.org/D12505" \
target="_blank">http://reviews.llvm.org/D12505</a></div><div><br></div><div>Thanks \
for the comments!</div><div>-Ahmed</div><div> </div><blockquote class="gmail_quote" \
style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div \
dir="ltr"><span><font \
color="#888888"><div>-eric</div></font></span></div><div><div><br><div \
class="gmail_quote"><div dir="ltr">On Thu, Aug 27, 2015 at 3:31 PM Ahmed Bougacha via \
cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" \
target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: \
ab<br>
Date: Thu Aug 27 17:30:38 2015<br>
New Revision: 246229<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=246229&view=rev" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=246229&view=rev</a><br>
Log:<br>
[X86] Conditionalize Darwin MaxVectorAlign on the presence of AVX.<br>
<br>
There's no point in using a larger alignment if we have no instructions<br>
that would benefit from it.<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D12389" rel="noreferrer" \
target="_blank">http://reviews.llvm.org/D12389</a><br> <br>
Modified:<br>
cfe/trunk/lib/Basic/Targets.cpp<br>
cfe/trunk/test/CodeGen/vector-alignment.c<br>
<br>
Modified: cfe/trunk/lib/Basic/Targets.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=246229&r1=246228&r2=246229&view=diff" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=246229&r1=246228&r2=246229&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Basic/Targets.cpp (original)<br>
+++ cfe/trunk/lib/Basic/Targets.cpp Thu Aug 27 17:30:38 2015<br>
@@ -3629,13 +3629,21 @@ public:<br>
LongDoubleWidth = 128;<br>
LongDoubleAlign = 128;<br>
SuitableAlign = 128;<br>
- MaxVectorAlign = 256;<br>
SizeType = UnsignedLong;<br>
IntPtrType = SignedLong;<br>
DataLayoutString = \
"e-m:o-p:32:32-f64:32:64-f80:128-n8:16:32-S128";<br> HasAlignMac68kSupport \
= true;<br> }<br>
<br>
+ bool handleTargetFeatures(std::vector<std::string> &Features,<br>
+ DiagnosticsEngine &Diags) override \
{<br> + if (!DarwinTargetInfo<X86_32TargetInfo>::handleTargetFeatures(Features,<br>
+ \
Diags))<br> + return false;<br>
+ // Now that we know if we have AVX, we can decide how to align vectors.<br>
+ MaxVectorAlign = hasFeature("avx") ? 256 : 128;<br>
+ return true;<br>
+ }<br>
};<br>
<br>
// x86-32 Windows target<br>
@@ -3986,13 +3994,22 @@ public:<br>
DarwinX86_64TargetInfo(const llvm::Triple &Triple)<br>
: DarwinTargetInfo<X86_64TargetInfo>(Triple) {<br>
Int64Type = SignedLongLong;<br>
- MaxVectorAlign = 256;<br>
// The 64-bit iOS simulator uses the builtin bool type for Objective-C.<br>
llvm::Triple T = llvm::Triple(Triple);<br>
if (T.isiOS())<br>
UseSignedCharForObjCBool = false;<br>
DataLayoutString = "e-m:o-i64:64-f80:128-n8:16:32:64-S128";<br>
}<br>
+<br>
+ bool handleTargetFeatures(std::vector<std::string> &Features,<br>
+ DiagnosticsEngine &Diags) override \
{<br> + if (!DarwinTargetInfo<X86_64TargetInfo>::handleTargetFeatures(Features,<br>
+ \
Diags))<br> + return false;<br>
+ // Now that we know if we have AVX, we can decide how to align vectors.<br>
+ MaxVectorAlign = hasFeature("avx") ? 256 : 128;<br>
+ return true;<br>
+ }<br>
};<br>
<br>
class OpenBSDX86_64TargetInfo : public OpenBSDTargetInfo<X86_64TargetInfo> \
{<br> <br>
Modified: cfe/trunk/test/CodeGen/vector-alignment.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/vector-alignment.c?rev=246229&r1=246228&r2=246229&view=diff" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Co \
deGen/vector-alignment.c?rev=246229&r1=246228&r2=246229&view=diff</a><br> \
==============================================================================<br>
--- cfe/trunk/test/CodeGen/vector-alignment.c (original)<br>
+++ cfe/trunk/test/CodeGen/vector-alignment.c Thu Aug 27 17:30:38 2015<br>
@@ -1,38 +1,51 @@<br>
-// RUN: %clang_cc1 -w -triple x86_64-apple-darwin10 -emit-llvm -o - %s | FileCheck \
%s<br> +// RUN: %clang_cc1 -w -triple x86_64-apple-darwin10 \<br>
+// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=ALL \
--check-prefix=SSE<br> +// RUN: %clang_cc1 -w -triple i386-apple-darwin10 \<br>
+// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=ALL \
--check-prefix=SSE<br> +// RUN: %clang_cc1 -w -triple x86_64-apple-darwin10 \
-target-feature +avx \<br> +// RUN: -emit-llvm -o - %s | FileCheck %s \
--check-prefix=ALL --check-prefix=AVX<br> +// RUN: %clang_cc1 -w -triple \
i386-apple-darwin10 -target-feature +avx \<br> +// RUN: -emit-llvm -o - %s | \
FileCheck %s --check-prefix=ALL --check-prefix=AVX<br> // rdar://11759609<br>
<br>
// At or below target max alignment with no aligned attribute should align \
based<br> // on the size of vector.<br>
double __attribute__((vector_size(16))) v1;<br>
-// CHECK: @v1 {{.*}}, align 16<br>
+// SSE: @v1 {{.*}}, align 16<br>
+// AVX: @v1 {{.*}}, align 16<br>
double __attribute__((vector_size(32))) v2;<br>
-// CHECK: @v2 {{.*}}, align 32<br>
+// SSE: @v2 {{.*}}, align 16<br>
+// AVX: @v2 {{.*}}, align 32<br>
<br>
// Alignment above target max alignment with no aligned attribute should align<br>
// based on the target max.<br>
double __attribute__((vector_size(64))) v3;<br>
-// CHECK: @v3 {{.*}}, align 32<br>
+// SSE: @v3 {{.*}}, align 16<br>
+// AVX: @v3 {{.*}}, align 32<br>
double __attribute__((vector_size(1024))) v4;<br>
-// CHECK: @v4 {{.*}}, align 32<br>
+// SSE: @v4 {{.*}}, align 16<br>
+// AVX: @v4 {{.*}}, align 32<br>
<br>
// Aliged attribute should always override.<br>
double __attribute__((vector_size(16), aligned(16))) v5;<br>
-// CHECK: @v5 {{.*}}, align 16<br>
+// ALL: @v5 {{.*}}, align 16<br>
double __attribute__((vector_size(16), aligned(64))) v6;<br>
-// CHECK: @v6 {{.*}}, align 64<br>
+// ALL: @v6 {{.*}}, align 64<br>
double __attribute__((vector_size(32), aligned(16))) v7;<br>
-// CHECK: @v7 {{.*}}, align 16<br>
+// ALL: @v7 {{.*}}, align 16<br>
double __attribute__((vector_size(32), aligned(64))) v8;<br>
-// CHECK: @v8 {{.*}}, align 64<br>
+// ALL: @v8 {{.*}}, align 64<br>
<br>
// Check non-power of 2 widths.<br>
double __attribute__((vector_size(24))) v9;<br>
-// CHECK: @v9 {{.*}}, align 32<br>
+// SSE: @v9 {{.*}}, align 16<br>
+// AVX: @v9 {{.*}}, align 32<br>
double __attribute__((vector_size(40))) v10;<br>
-// CHECK: @v10 {{.*}}, align 32<br>
+// SSE: @v10 {{.*}}, align 16<br>
+// AVX: @v10 {{.*}}, align 32<br>
<br>
// Check non-power of 2 widths with aligned attribute.<br>
double __attribute__((vector_size(24), aligned(64))) v11;<br>
-// CHECK: @v11 {{.*}}, align 64<br>
+// ALL: @v11 {{.*}}, align 64<br>
double __attribute__((vector_size(80), aligned(16))) v12;<br>
-// CHECK: @v12 {{.*}}, align 16<br>
+// ALL: @v12 {{.*}}, align 16<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" \
target="_blank">cfe-commits@lists.llvm.org</a><br> <a \
href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" \
target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br> \
</blockquote></div> </div></div></blockquote></div><br></div></div>
[Attachment #6 (text/plain)]
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic