bjope added inline comments.
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3208
+ if (LHSWidth < CommonWidth)
+ LHS = LHSSign ? Builder.CreateSExt(LHS, CommonTy)
+ : Builder.CreateZExt(LHS, CommonTy);
----------------
`LHS = Builder.CreateIntCast(LHS, CommonTy, LHSSign);`
(I think that it even is possible to remove the condition and always do this
(the cast will be a no-op if LHSWidth==CommonWidth))
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3211
+ if (RHSWidth < CommonWidth)
+ RHS = RHSSign ? Builder.CreateSExt(RHS, CommonTy)
+ : Builder.CreateZExt(RHS, CommonTy);
----------------
Same as above, this can be simplified as:
RHS = Builder.CreateIntCast(RHS, CommonTy, RHSSign);
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3214
+
+ // Align scales
+ unsigned CommonScale = std::max({LHSScale, RHSScale, ResultScale});
----------------
Arithmetically I think that it would be enough to align the scale to
```
std::max(std::min(LHSScale, RHSScale), ResultScale)
```
As adding low fractional bits outside the result precision, when we know that
those bits are zero in either of the inputs, won't impact any more significant
bits in the result.
Maybe your solution is easier and good enough for a first implementation. And
maybe `opt` is smart enough to optimize this anyway. Or maybe codegen could be
worse due to not using legal types. Or maybe codegen could be improved due to
using sadd.sat in more cases?
Lots of "maybes", so I don't think you need to do anything about this right
now. It is just an idea from my side.
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3252
+ llvm::Function *intrinsic = CGF.CGM.getIntrinsic(IID, CommonTy);
+ Result = Builder.CreateCall(intrinsic, {LHS, RHS});
+ }
----------------
It should be possible to do it like this (if you add some line wrapping):
```
Builder.CreateBinaryIntrinsic(ResultSign ? llvm::Intrinsic::sadd_sat :
llvm::Intrinsic::uadd_sat, LHS, RHS);
```
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
