[PATCH] Fix for Bug 5053

[Swaroop Sridhar]

Thanks a lot for the review Reid.

1) I'm using VS2013, LLVM 3.5, and not using MCJIT to repro this failure.
2) I've updated the test case to look like the actual test reported in Bug 5053.
     The problem should repro now, can you please try again? 
3) I've fixed the issues you've identified.
    -> Fixed the wording in the comments for the test case 
    -> Made copysignf VS2013 only
4) In DynamicLibrary.inc, I've simplified the implementation of the inline_ *() functions.
    Now, inline_fmodf(x,y) simply calls fmodf(x,y) instead of performing (float)fmod(x,y)

I've attached a new patch for with the updates. Please take a look at it.

Thanks,
Swaroop.


-----Original Message-----
From: Reid Kleckner [mailto:rnk at google.com] 
Sent: Thursday, October 2, 2014 10:57 AM
To: Swaroop Sridhar; bigcheesegs at gmail.com; anton at korobeynikov.info
Cc: rnk at google.com; llvm-commits at cs.uiuc.edu
Subject: Re: [PATCH] Fix for Bug 5053

Your test case already passes for me. Can you elaborate on when this fails and in what environment, i.e. VS version, LLVM version, MCJIT vs interpreter, etc?

================
Comment at: lib/Support/Windows/DynamicLibrary.inc:138
@@ -121,1 +137,3 @@
 #undef EXPLICIT_SYMBOL2
+#undef INLINE_DEF_SYMBOL
+#undef INLINE_DEF_SYMBOL2
----------------
This should either be SYMBOL1 or the define should drop the trailing 1.

================
Comment at: lib/Support/Windows/explicit_symbols.inc:74
@@ +73,3 @@
+  INLINE_DEF_FLOAT_VERSION(ceil, 1)
+  INLINE_DEF_FLOAT_VERSION(copysign, 2)  INLINE_DEF_FLOAT_VERSION(cos, 
+ 1)
----------------
What version of VS are you testing with? I don't believe this will compile on VS 2012, because it only has _copysign. See the 2012 MSDN docs vs the 2013 docs:
http://msdn.microsoft.com/en-us/library/0yafk1hc(v=vs.110).aspx
http://msdn.microsoft.com/en-us/library/0yafk1hc(v=vs.120).aspx

Furthermore, with VS2013, I think we can simply use the EXPLICIT_SYMBOL mechanism instead of a new approach.

================
Comment at: test/CodeGen/X86/win_frem.ll:1 @@ +1,2 @@
+; This unit test tests LLI.exe crashes on Windows\X86 when certain 
+single precession ; floating point intrinsics are used
----------------
With the fix committed, this should be "used to crash" instead of "crashes".

Also, this test is an execution test that belongs in test/ExecutionEngine instead of test/CodeGen/X86.

http://reviews.llvm.org/D5387


-------------- next part --------------
A non-text attachment was scrubbed...
Name: inline-frem.patch
Type: application/octet-stream
Size: 6448 bytes
Desc: inline-frem.patch
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141011/5745cdde/attachment.obj>