ebevhan added a comment. In https://reviews.llvm.org/D53738#1305498, @leonardchan wrote:
> @ebevhan Any more comments on this patch? It's strange, I feel like I've responded to the last comment twice now but there's nothing in Phab. Generally I think it's good! One final note; I assume we could technically reuse/rename the EmitFixedPointAdd function and use it to emit other binops when those are added? ================ Comment at: clang/test/Frontend/fixed_point_add.c:269 + // UNSIGNED-NEXT: [[SUM:%[0-9]+]] = call i15 @llvm.uadd.sat.i15(i15 [[USA_TRUNC]], i15 [[USA_SAT_TRUNC]]) + // UNSIGNED-NEXT: [[SUM_EXT:%[a-z0-9]+]] = zext i15 [[SUM]] to i16 + // UNSIGNED-NEXT: store i16 [[SUM_EXT]], i16* %usa_sat, align 2 ---------------- leonardchan wrote: > ebevhan wrote: > > This is probably a candidate for an isel optimization. This operation also > > works as an `i16 ssat.add` with a negative-clamp-to-zero afterwards, and if > > the target supports `i16 ssat.add` natively then it will likely be a lot > > more efficient than whatever an `i15 uadd.sat` produces. > Do you think it would be more efficient for now then if instead we did SHL by > 1, saturate, then [AL]SHR by 1? This way we could use `i16 ssat.add` instead > of `i15 ssat.add`? We should probably just do it in isel or instcombine instead. We don't know at this point which intrinsic is a better choice (though, I think power-of-two-types are generally better). Repository: rC Clang https://reviews.llvm.org/D53738 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
