<div dir="ltr"><div><div><div>After some architecture testing, it looks like we can drop the march options and just enable the specific instruction sets via:<br><br></div>libswrAVX: -mavx<br></div>libswrAVX2: -mavx2 -mfma -mbmi2 -mf16c<br><br></div>The benefit of course is that with these flags swr works on AMD cpu's with AVX and AVX2 instructions enabled.  The specific march flags can cause instruction set compatibility mismatch since AMD CPUs that supported AVX didn't necessarily support all of sandybridge instructions and similar for AVX2 and haswell.  Regarding the extra options for fma, bmi2, and f16c, all AMD CPUs that supported AVX2 also supported those as well.<br></div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr">- Chuck<br></div></div></div>
<br><div class="gmail_quote">On Mon, Jun 13, 2016 at 1:22 PM, Emil Velikov <span dir="ltr"><<a href="mailto:emil.l.velikov@gmail.com" target="_blank">emil.l.velikov@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 13 June 2016 at 15:43, Rowley, Timothy O <<a href="mailto:timothy.o.rowley@intel.com">timothy.o.rowley@intel.com</a>> wrote:<br>
><br>
>> On Jun 12, 2016, at 3:56 AM, Steven Newbury <<a href="mailto:steve@snewbury.org.uk">steve@snewbury.org.uk</a>> wrote:<br>
>><br>
>> On Fri, 2016-06-10 at 16:05 -0400, Ilia Mirkin wrote:<br>
>>> On Fri, Jun 10, 2016 at 3:43 PM, Tim Rowley <timothy.o.rowley@intel.c<br>
>>> om> wrote:<br>
>>>><br>
>>>> Previously used core-avx-i was for ivybridge;<br>
>>>> corei7-avx allows sandybridge.<br>
>>>> ---<br>
>>>>  src/gallium/drivers/swr/Makefile.am | 2 +-<br>
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)<br>
>>>><br>
>>>> diff --git a/src/gallium/drivers/swr/Makefile.am<br>
>>>> b/src/gallium/drivers/swr/Makefile.am<br>
>>>> index d211f2e..8156cf2 100644<br>
>>>> --- a/src/gallium/drivers/swr/Makefile.am<br>
>>>> +++ b/src/gallium/drivers/swr/Makefile.am<br>
>>>> @@ -124,7 +124,7 @@ COMMON_LDFLAGS = \<br>
>>>>  lib_LTLIBRARIES = libswrAVX.la libswrAVX2.la<br>
>>>><br>
>>>>  libswrAVX_la_CXXFLAGS = \<br>
>>>> -       -march=core-avx-i \<br>
>>>> +       -march=corei7-avx \<br>
>>> Just wondering if it'd be enough to say like<br>
>>><br>
>>> -march=x86_64<br>
>>> -mavx<br>
>>><br>
>>> and add -mavx2 for libswrAVX2.<br>
>>><br>
>>> I suspect you've iterated through this 20 times and this has some<br>
>>> shortcoming I'm not thinking of, but figured I'd point it out just in<br>
>>> case it helps.<br>
>>><br>
>>>   -ilia<br>
>>><br>
>> Maybe I'm the only one who finds it horrible to override -march from<br>
>> within project build systems.<br>
<br>
>> It causes no end of problems with LTO,<br>
>> and results in objects being built inappropriately for the target as<br>
>> specified by the builder.<br>
</div></div>Can you please elaborate on the exact problem ? Is there link where<br>
one can read more on the topic ?<br>
<span class=""><br>
> I agree that the way swr is built can be improved and welcome ideas/discussion for work on the master branch, but for the moment I’m trying to get the 12.0 branch in good shape for swr with non-invasive changes (as we’ve tested the current setup pretty extensively).<br>
><br>
</span>Fully agree. Getting something more robust and less evasive for 12.0<br>
and work for 'the best' in master. Perhaps attribute((target)) perhaps<br>
folding the multiple binaries into one, or maybe something else ?<br>
<span class=""><br>
>> Why not use function attributes?  Either compile a function<br>
>> specifically for a particular target, or specifically enable AVX though<br>
>> "__attribute__ ((__target__ ("avx")))"<br>
</span>Is this going to produce correct binary if one provides -mno-avx<br>
and/or alike ? I believe for some CPUs one should (or the compiler<br>
does it for them) explicitly disable avx but honestly I've never<br>
looked if cpuid for the said CPUs shows avx as supported.<br>
<br>
In the latter case (cpuid listing AVX as supported, while one is<br>
recommended building with -mno-avx) how do you suggest we should<br>
handle things ?<br>
<span class=""><br>
>> (ideally with a fall-back<br>
>> implementation detected at run-time when the instruction set isn't<br>
>> supported).  Even better, have it also protected at build-time by a<br>
>> simple check for __AVX__ so the code can be compiled out entirely where<br>
>> the package builder has specified -march(=native) as a build flag.<br>
</span>The idea here is to have the binary build explicitly with avx/avx2 (if<br>
compiler is capable), even when doing generic builds. The picky part<br>
comes when one has to establish if the user provided one is 'more<br>
generic' or not (as per your example).<br>
<br>
Then again, based on my limited 'testing', building mesa with native<br>
and LTO brings us little to no improvements. 'Tested' running glxgears<br>
in small window.<br>
<span class=""><br>
> One problem with the function attribute path is that as far as I know, MSVC does not have a similar feature.<br>
><br>
</span>Afaict MSVC has optimise pragma [1], just like GCC. Ideally they'll<br>
add an equivalent to attribute((target)) soon ?<br>
<br>
Even without that, swr (as seen in mesa) does not build MSVC. When<br>
that cases it should be a matter of factoring the attribute target as<br>
a macro and leaving it empty for the !GCC build ?<br>
<br>
TL;DR: IMHO fix is ok for now, but for the future we might want to<br>
explode other options.<br>
<br>
-Emil<br>
[1] <a href="https://msdn.microsoft.com/en-us/library/chh3fb0k.aspx" rel="noreferrer" target="_blank">https://msdn.microsoft.com/en-us/library/chh3fb0k.aspx</a><br>
<div class="HOEnZb"><div class="h5">_______________________________________________<br>
mesa-stable mailing list<br>
<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-stable" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-stable</a><br>
</div></div></blockquote></div><br></div>