<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Aug 31, 2015 at 4:03 PM Ahmed Bougacha <<a href="mailto:ahmed.bougacha@gmail.com">ahmed.bougacha@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><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><div dir="ltr"><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></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div></div></div></div></blockquote><div><br></div><div>This is harder. I mean, right now we depend ABI based on the command line options for the whole file which seems reasonable from a calling convention standpoint. I think right now you could construct an unaligned vector testcase and use it with a builtin?</div><div><br></div><div>I think we can get it to where you can look at a function and get the proper feature set for it in short order - I'm working on that anyhow, right now it would be messy though.</div><div><br></div><div>What would you like to do about this right now though?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><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"><div>Also, should we do this for all of the x86 OSes?</div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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></div></div></blockquote><div><br></div><div>I've ack'd that. Thanks for doing it.</div><div><br></div><div>-eric</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div>Thanks for the comments!</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>-Ahmed</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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/CodeGen/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></div></div></blockquote></div></div>