detect FP contraction at test-time, disable certain subtests if detected

[Abe]

Summary

The program in the new file [when compiled with Clang] outputs "(1 1 1) (1 1) (1)" when no FP contraction is detected, and one or more of those 1s is replaced with a 0 when some FP contraction is detected.

Note: the new program is not guaranteed to always perfectly detect contraction with all compilers [only Clang and GCC were tested by Abe]. This is a best-effort approach that is as portable as Abe knows how to make it. When the detection program errs, it should err on the side of reporting a 1 when it should have reported a zero. Reporting a 0 when it should have reported a 1 should not be possible.

I [Abe] would appreciate help/suggestions on how to make the Makefile test harness run this program [prefereably only once per test run] and capture the result in a variable. Although I am quite proficient in GNU Make, I`m unsure how it is being [mis]used as a test harness for the LLVM "test-suite".

Added Oct. 17 2016, as per suggestion by Matthias: the program now returns an exit code of 0 only if no contraction was detected, 1 if any was.

[Abe]

added support for GCC [4.4 or higher required AFAIK]

[MatzeB]

I am loosing track of the discussion as to why this is necessary. The big problem here however is cross compilation: you cannot assume that you can actually execute the results of this program as part of the build system configuration.

tools/FP-contraction_detection.c

37–41

AFAIK this is violating strict aliasing rules depending on which version of the C standard you use. I don't think any compiler would dare to miscompile it because the pattern is in widespread use. But correct way to do it however is something like:

static double u64_to_double(uint64_t x) {
  double d;
  memcpy(&d, &x, sizeof(x));
  return d;
}

48–53

I am not sure how exactly this is intended to be used, but usually it is far friendlier for scripting if you just do "return 0" or "return 1" in the main function. You could still have commandline switches if you really need multiple criteria or want a verbose mode with details on stdout/stderr.

[Abe]

implemented 2 good suggestions from MB ["u64_to_double" instead of union, exit code]

[rengolin]

In D25635#570957, @MatzeB wrote:

I am loosing track of the discussion as to why this is necessary. The big problem here however is cross compilation: you cannot assume that you can actually execute the results of this program as part of the build system configuration.

I'm coming late to this discussion, sorry, I stopped getting emails from Phab for a while (no idea why).

I don't think that cross-compilation is a problem here. This is the test-suite (not LLVM), so we have to assume the compiled code runs on whatever target they were meant for.

I think this tool is interesting, but I'm not sure how we could use it in the current test-suite. Maybe Abe could expand on how we're expecting to use this on the remaining makefiles.

I have a number of inline comments, but mostly about style.

cheers,
--renato

tools/FP-contraction_detection.c

9

I'd rather have less verbose method names. My brain stops reading after the second underscore (or caps change).

Simpler names would be:

ContractDefaultSettings();
ContractPragmaNo();
ContractPragmaYes();
ContractOptNone();

45

This link needs a login. Please either explain what it is or find another link.

48–53

I agree and I was going to reply to that effect.

We can have both debug dumps and return values, but returning 0 for success and non-0 for failure is the accepted standard.

55

Since you're going to print stuff, why not be clear and verbose?

Since you're re-using the comparison below, why not give it a clear name, like:

FP_type Default = FMA_test_using_default_compiler_setting(A, B, C);
FP_type NoContract = FMA_test_with_pragma_for_NO_contraction(A, B, C);
FP_type YesContract = FMA_test_with_pragma_for_YES_contraction(A, B, C);
...
FP_type DefaultNoContract = (Default == NoContract);
FP_type DefaultYesContract = (Default == YesContract);
...
if (DEBUG) {
  printf("Default contract is: %s", (DefaultNoContract ? "No" : "Yes");
  ...
}
return ! (Default == NoContract == YesContract == NoOpt);

[MatzeB]

In D25635#622006, @rengolin wrote:

In D25635#570957, @MatzeB wrote:

I am loosing track of the discussion as to why this is necessary. The big problem here however is cross compilation: you cannot assume that you can actually execute the results of this program as part of the build system configuration.

I'm coming late to this discussion, sorry, I stopped getting emails from Phab for a while (no idea why).

I don't think that cross-compilation is a problem here. This is the test-suite (not LLVM), so we have to assume the compiled code runs on whatever target they were meant for.

No! Experience shows that different people require various crazy combinations of simulators, ssh ing into a remote device, setting odd environment variables and other magic incantations to actually run target code when crosscompiling.

If possible I'd like to keep this logic out of the buildsystem and contained withing the lit runner. That means not running target code during the build.

[rengolin]

In D25635#622486, @MatzeB wrote:

No! Experience shows that different people require various crazy combinations of simulators, ssh ing into a remote device, setting odd environment variables and other magic incantations to actually run target code when crosscompiling.

Hum, you're right. I remember the test-suite used the compiler options for almost everything, so I assumed it would compile the tools with the same compiler, but that's obviously wrong.

Indeed, this makes it a no-starter for this patch. :(

--renato

tools/CMakeLists.txt

add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/timeit
     COMMAND cp ${CMAKE_CURRENT_SOURCE_DIR}/timeit.sh ${CMAKE_CURRENT_BINARY_DIR}/timeit
     COMMAND chmod +x ${CMAKE_CURRENT_BINARY_DIR}/timeit
     DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/timeit.sh)
   add_custom_target(timeit-host DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/timeit)
 else()
   add_executable(timeit-target ${CMAKE_CURRENT_SOURCE_DIR}/timeit.c)
   llvm_add_host_executable(timeit-host timeit timeit.c)
 endif()
+
+add_executable(FP-contraction_detection___-target ${CMAKE_CURRENT_SOURCE_DIR}/FP-contraction_detection.c)

tools/FP-contraction_detection.c

+#include <memory.h>
+#include <stdio.h>
+#include <stdint.h>
+
+// for ease of future change
+#define FP_type double
+
+__attribute__((noinline))
+FP_type FMA_test_using_default_compiler_setting( FP_type a, FP_type b, FP_type c) {
+  return a+b*c;
+}
+
+__attribute__((noinline))
+FP_type FMA_test_with_pragma_for_NO_contraction( FP_type a, FP_type b, FP_type c) {
+  #pragma STDC FP_CONTRACT OFF
+  return a+b*c;
+}
+
+__attribute__((noinline))
+FP_type FMA_test_with_pragma_for_YES_contraction(FP_type a, FP_type b, FP_type c) {
+  #pragma STDC FP_CONTRACT ON
+  return a+b*c;
+}
+
+#pragma clang optimize off
+__attribute__((optnone))         // for Clang
+__attribute__((optimize("-O0"))) // for GCC
+__attribute__((noinline))
+FP_type FMA_test_trying_to_disable_optimization( FP_type a, FP_type b, FP_type c) {
+  return a+b*c;
+}
+
+
+// the following routine: with thanks to Matthias Braun
+static double u64_to_double(uint64_t x) {
+  double d;
+  memcpy(&d, &x, sizeof(x));
+  return d;
+}
+
+
+int main() {
+
+  double A, B, C;
+  // the next 3 lines: thanks to the author of <https://www.pgroup.com/lit/articles/insider/v3n3a4.htm>
+  A = u64_to_double(0x3c54c9b71a0e6500);
+  B = u64_to_double(0xbf43a04556d864ae);
+  C = u64_to_double(0xbfc55364b6b08299);
+
+  FP_type result_1 = FMA_test_using_default_compiler_setting( A, B, C);
+  FP_type result_2 = FMA_test_with_pragma_for_NO_contraction( A, B, C);
+  FP_type result_3 = FMA_test_with_pragma_for_YES_contraction(A, B, C);
+  FP_type result_4 = FMA_test_trying_to_disable_optimization( A, B, C);
+
+  printf("(%d "  , result_1 == result_2);
+  printf( "%d "  , result_1 ==             result_3);
+  printf( "%d) " , result_1 ==                         result_4);
+  printf("(%d "  ,             result_2 == result_3);
+  printf( "%d) " ,             result_2 ==             result_4);
+  printf("(%d)\n",                         result_3 == result_4);
+
+  return ! ( (result_1 == result_2) && (result_1 == result_3) && (result_1 == result_4) &&
+             (result_2 == result_3) && (result_2 == result_4) &&
+             (result_3 == result_4)
+           );
+}

tools/Makefile

timeit: timeit.sh
   cp -f $< $@
   chmod u+x $@
 
 timeit-target: timeit.sh
   cp -f $< $@
   chmod u+x $@
 endif
 
+FP-contraction_detection___-target: FP-contraction_detection.c
+  $(LD_ENV_OVERRIDES) $(LCC) -o $@ $< $(LDFLAGS) $(CFLAGS) $(TARGET_FLAGS) -O3
+
 fpcmp: fpcmp.c
   $(ORIGINAL_CC) $(CFLAGS) -O3 -o $@ $<
 
 TargetConfig.mk: TargetConfig.mk.in
   $(LCC) $(LDFLAGS) $(CFLAGS) $(TARGET_FLAGS) -E -o $@ -x c $<
 
 clean::
   rm -f timeit timeit-target fpcmp TargetConfig.mk