<div dir="ltr"><div>Since the idea here is to correctly log a user-generated error (user-cpuid > detected cpuid), the patch is headed in the right direction. <br><br></div>Min's suggestion on using an AND mask sounds good, and can you also make the warning more informative (print user-cpuid, and the cpuid we're defaulting to) ?<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Oct 29, 2015 at 11:16 PM, Steve Borho <span dir="ltr"><<a href="mailto:steve@borho.org" target="_blank">steve@borho.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 10/28, <a href="mailto:dnyaneshwar@multicorewareinc.com">dnyaneshwar@multicorewareinc.com</a> wrote:<br>
> # HG changeset patch<br>
> # User Dnyaneshwar G <<a href="mailto:dnyaneshwar@multicorewareinc.com">dnyaneshwar@multicorewareinc.com</a>><br>
> # Date 1446021877 -19800<br>
> #      Wed Oct 28 14:14:37 2015 +0530<br>
> # Node ID 975087370d14e90cd63edecb34fb4bf2feda2468<br>
> # Parent  6563218ce342c30bfd4f9bc172a1dab510e6e55b<br>
> fix invalid Instruction Set provided in CLI if CPU doesn't support it<br>
><br>
> This patch avoids crash/invalid instructions when we provide instruction sets to<br>
> be used are higher than the cpu capabilities.<br>
><br>
> For example, if our cpu supports instruction sets upto AVX and we provide<br>
> --asm "avx2" (AVX2 is higher than AVX) then it will show warning and use default<br>
> x265 detected intruction sets.<br>
<br>
</span>The whole point of having this override is in case our CPU detection is<br>
somehow wrong. The user needs to be able to override the detection mask.<br>
<br>
That said.. if the user provided mask has bits set that were not<br>
detected, it's ok to log a serious warning that says you think the<br>
encoder is about to break and it is the user's fault.<br>
<br>
BTW: this feature is often used for benchmarking, to disable certain<br>
optimizations piecemeal, but that is not the primary reason why it<br>
exists.<br>
<span class="im HOEnZb"><br>
> diff -r 6563218ce342 -r 975087370d14 source/common/primitives.cpp<br>
> --- a/source/common/primitives.cpp    Mon Oct 26 12:13:53 2015 +0530<br>
> +++ b/source/common/primitives.cpp    Wed Oct 28 14:14:37 2015 +0530<br>
> @@ -238,6 +238,15 @@<br>
>              <a href="http://primitives.cu" rel="noreferrer" target="_blank">primitives.cu</a>[i].intra_pred_allangs = NULL;<br>
><br>
>  #if ENABLE_ASSEMBLY<br>
> +<br>
> +        if ((uint32_t)param->cpuid > X265_NS::cpu_detect())<br>
> +        {<br>
> +            if (param->logLevel >= X265_LOG_INFO)<br>
> +                x265_log(param, X265_LOG_WARNING, "Unsupported CPUID provided in CLI, so choosing x265 detected CPUID!\n");<br>
> +<br>
> +            param->cpuid = X265_NS::cpu_detect();<br>
> +        }<br>
> +<br>
>          setupInstrinsicPrimitives(primitives, param->cpuid);<br>
>          setupAssemblyPrimitives(primitives, param->cpuid);<br>
>  #endif<br>
> _______________________________________________<br>
> x265-devel mailing list<br>
> <a href="mailto:x265-devel@videolan.org">x265-devel@videolan.org</a><br>
> <a href="https://mailman.videolan.org/listinfo/x265-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/x265-devel</a><br>
<br>
</span><span class="HOEnZb"><font color="#888888">--<br>
Steve Borho<br>
</font></span><div class="HOEnZb"><div class="h5">_______________________________________________<br>
x265-devel mailing list<br>
<a href="mailto:x265-devel@videolan.org">x265-devel@videolan.org</a><br>
<a href="https://mailman.videolan.org/listinfo/x265-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/x265-devel</a><br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature"><div dir="ltr"><div><div>Deepthi Nandakumar<br></div>Engineering Manager, x265<br></div>Multicoreware, Inc<br></div></div>
</div>