diff mbox

Bug 201689: Belt-and-suspenders detection of FMA.

Message ID 90af3efa-aaca-2dce-e433-1df7e5dbcfbd@redhat.com
State New
Headers show

Commit Message

Carlos O'Donell Oct. 14, 2016, 6:09 p.m. UTC 
In the Intel Architecture Instruction Set Extensions Programming reference
the recommended way to test for FMA in section '2.2.1 Detection of FMA'
is:

"Application Software must identify that hardware supports AVX as explained
 in ... after that it must also detect support for FMA..."

We don't do that in glibc. We use osxsave to detect the use of xgetbv, and
after that we check for AVX and FMA orthogonally. It is conceivable that
you could have the AVX bit clear and the FMA bit in an undefined state.

I have never seen a machine with the AVX bit clear and the FMA bit set, but
we should follow the intel specifications and adjust our check as the following
patch works.

OK to checkin?

2016-10-14  Carlos O'Donell  <carlos@redhat.com>

	[BZ #20689]
	* sysdeps/x86/cpu-features.c: Only enable FMA is AVX is present.

---

Comments

H.J. Lu Oct. 14, 2016, 7:12 p.m. UTC | #1 
On Fri, Oct 14, 2016 at 11:09 AM, Carlos O'Donell <carlos@redhat.com> wrote:
> In the Intel Architecture Instruction Set Extensions Programming reference
> the recommended way to test for FMA in section '2.2.1 Detection of FMA'
> is:
>
> "Application Software must identify that hardware supports AVX as explained
>  in ... after that it must also detect support for FMA..."
>
> We don't do that in glibc. We use osxsave to detect the use of xgetbv, and
> after that we check for AVX and FMA orthogonally. It is conceivable that
> you could have the AVX bit clear and the FMA bit in an undefined state.
>
> I have never seen a machine with the AVX bit clear and the FMA bit set, but
> we should follow the intel specifications and adjust our check as the following
> patch works.

One can't write a program with FMA nor AVX2 without using AVX
instructions.  AVX2/FMA aren't usable if AVX isn't.   We should do

if (CPU_FEATURES_CPU_P (cpu_features, AVX))
   {
      Set AVX available
      if (CPU_FEATURES_CPU_P (cpu_features, AVX2))
         Set AVX2 available
      if (CPU_FEATURES_CPU_P (cpu_features, FMA))
         Set FMA available
   }

> OK to checkin?
>
> 2016-10-14  Carlos O'Donell  <carlos@redhat.com>
>
>         [BZ #20689]
>         * sysdeps/x86/cpu-features.c: Only enable FMA is AVX is present.
>
> diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
> index 11b9af2..1d52f22 100644
> --- a/sysdeps/x86/cpu-features.c
> +++ b/sysdeps/x86/cpu-features.c
> @@ -83,8 +83,10 @@ get_common_indeces (struct cpu_features *cpu_features,
>                       |= bit_arch_AVX512DQ_Usable;
>                 }
>             }
> -         /* Determine if FMA is usable.  */
> -         if (CPU_FEATURES_CPU_P (cpu_features, FMA))
> +         /* Determine if FMA is usable.  The recommended Intel procedure
> +            is to check for AVX && FMA to decide if FMA is available.  */
> +         if (CPU_FEATURES_CPU_P (cpu_features, AVX)
> +             && CPU_FEATURES_CPU_P (cpu_features, FMA))
>             cpu_features->feature[index_arch_FMA_Usable]
>               |= bit_arch_FMA_Usable;
>         }
> ---
>
>
> --
> Cheers,
> Carlos.
diff mbox

Patch

diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
index 11b9af2..1d52f22 100644
--- a/sysdeps/x86/cpu-features.c
+++ b/sysdeps/x86/cpu-features.c
@@ -83,8 +83,10 @@  get_common_indeces (struct cpu_features *cpu_features,
                      |= bit_arch_AVX512DQ_Usable;
                }
            }
-         /* Determine if FMA is usable.  */
-         if (CPU_FEATURES_CPU_P (cpu_features, FMA))
+         /* Determine if FMA is usable.  The recommended Intel procedure
+            is to check for AVX && FMA to decide if FMA is available.  */
+         if (CPU_FEATURES_CPU_P (cpu_features, AVX)
+             && CPU_FEATURES_CPU_P (cpu_features, FMA))
            cpu_features->feature[index_arch_FMA_Usable]
              |= bit_arch_FMA_Usable;
        }