ebevhan added inline comments.

================
Comment at: llvm/include/llvm/IR/FixedPointBuilder.h:171-172
+    // lossless, except for overflow to infinity which is unlikely.
+    return B.CreateFMul(Result,
+        ConstantFP::get(DstTy, std::pow(2, -(int)SrcSema.getScale())));
+  }
----------------
leonardchan wrote:
> Was there a reason for preferring multiplying by the reciprocal instead of 
> dividing by a power of 2?
> 
> I think this was discussed before, but can't seem to remember/find the 
> conversation.
Multiplication should be more efficient than division with the same effect.

However, as @rjmccall mentioned in D54749, this rescaling might not work if the 
exponent is not large enough to fit the scaled value. This is unlikely for 
single precision float and larger formats, but it probably won't work for half 
precision.

I'm not entirely sure what to do about that. I was originally planning on 
adding an intrinsic that did pretty much the same thing as the one in that 
patch, but with a scaling factor. However, I figured that it would not be 
necessary when I noticed that an integer intrinsic was already in the works.

It feels odd to have intrinsics that do pretty much the same thing as another 
intrinsic but with an internal multiplication baked in. On top of that, it 
would require adding 4 new intrinsics (signed/unsigned, 
nonsaturating/saturating) which seems like a bit much.

Perhaps it may be necessary to emit something else when half precision is 
involved here...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86632/new/

https://reviews.llvm.org/D86632

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to