az added inline comments.

================
Comment at: include/clang/Basic/arm_neon.td:1504
+  // Scalar floating point multiply extended (scalar, by element)
+  def SCALAR_FMULX_LANEH : IOpInst<"vmulx_lane", "ssdi", "Sh", 
OP_SCALAR_MUL_LN>;
+  def SCALAR_FMULX_LANEQH : IOpInst<"vmulx_laneq", "ssji", "Sh", 
OP_SCALAR_MUL_LN>;
----------------
SjoerdMeijer wrote:
> I found that unfortunately it's not that straightforward. This leads to wrong 
> code generation as it is generating a fmul instead of fmulx. I am suspecting 
> this instruction description should be using OP_SCALAR_MULX_LN, but also the 
> type decls are wrong. Need to dig a bit further here.
Sorry for confusion as the commented code was never intended to be used and it 
is a copy of the code for the intrinsic vmulh_lane(). It was done that way in 
order to point out that vmulh_lane() and vmulxh_lane() intrinsics should be 
implemented in a similar way. The only useful thing in the commented code is 
the explanation that we need the scalar intrinsic vmulxh_f16() which was 
implemented in the scalar intrinsic patch later on.
 
If we look at how vmulh_lane (a, b, lane) is implemented:
x = extract (b, lane);
res = a * x;
return res; 

Similarly, I thought at the time that vmulxh_lane (a, b, lane) can be 
implemented: 
x = extract (b, lane);
res = vmulxh_f16 (a, x);      // no llvm native mulx instruction, so we use the 
fp16 scalar intrinsic.
return res; 

I am not sure now that we can easily use scalar intrinsic while generating the 
arm_neon.h file. In case we can not do that, I am thinking that the frontend 
should generate a new builtin for intrinsic vmulxh_lane() that the backend 
recognizes and generate the right code for it which is fmulx  h0, h0, 
v1.h[lane]. If you made or will be making progress on this, then that is great. 
Otherwise, I can look at a frontend solution for it.  


https://reviews.llvm.org/D44222



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

Reply via email to