[PATCH] [ConstantFolding] Fix wrong folding of intrinsic 'convert.from.fp16'.

Andrea Di Biagio Andrea_DiBiagio at sn.scee.net
Thu May 14 09:18:46 PDT 2015


Thanks for the review!


================
Comment at: lib/Analysis/ConstantFolding.cpp:1403
@@ +1402,3 @@
+// constant folding intrinsic calls to 'convert_from_fp16'.
+static const fltSemantics &getFloatingPointSemanticFromType(const Type *Ty) {
+  if (Ty->isHalfTy())
----------------
ab wrote:
> I swear I saw the same construct elsewhere, but I can't find it.  Is there a good header where this could go?
You are right!
I should have use `getFltSemantics()` from Type.h.
I will use it :-).

================
Comment at: test/Transforms/ConstProp/convert-from-fp16.ll:3-4
@@ +2,4 @@
+
+; Verify that we don't crash with an assertion failure when constant folding
+; a call to intrinsic 'convert.from.fp16' if the return type is not 'float'.
+
----------------
ab wrote:
> Since this means the code was untested before, what using a non-0 value?  For the extension, the significand of the result should look the same, so we can get creative. (but there's no point in testing APFloat again;  it's just that 0 is easy to get right the wrong way)
Right, I can add more tests. My original test case had something like this:

```
  %0 = call i16 @llvm.convert.to.fp16.f64(double 5.25)
  %1 = call double @llvm.convert.from.fp16.f64(i16 %0)
  ret double %1
```

I can add more tests like the above example.
I'll send a new patch soon.

================
Comment at: test/Transforms/ConstProp/convert-from-fp16.ll:7
@@ +6,3 @@
+define float @fold_from_fp16_to_fp32() {
+; CHECK: ret float 0.000000e+00
+entry:
----------------
ab wrote:
> CHECK-LABEL?
I will add CHECK-LABELs.

http://reviews.llvm.org/D9771

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list