ebevhan added a comment.
I think I've mentioned this before, but I would like to discuss the possibility
of adding a target hook(s) for some of these operations (conversions,
arithmetic when it comes). Precisely matching the emitted saturation operation
is virtually impossible for a target.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:331
+ SourceLocation Loc);
+
/// Emit a conversion from the specified complex type to the specified
----------------
What's the plan for the other conversions (int<->fix, float<->fix)? Functions
for those as well?
What about `EmitScalarConversion`? If it cannot handle conversions of
fixed-point values it should probably be made to assert, since it will likely
mess up.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:1017
+ // Need to extend first before scaling up
+ ResultWidth = SrcWidth + DstScale - SrcScale;
+ llvm::Type *UpscaledTy = Builder.getIntNTy(ResultWidth);
----------------
It is definitely simpler to always do the 'upscale+resize/downscale,
[saturate], resize' in the constant evaluation case, but when emitting IR it is
not as efficient. There's no need to keep the upshifted bits if you aren't
interested in saturation, so in the non-saturating case it is better to keep
everything in the native type sizes with '[downscale], resize, [upscale]'. This
is why there are a bunch of 'sext i32 to i33' in the test cases.
Perhaps something like
```
if !dst.saturating
downscale if needed
resize
upscale if needed
return
old code here, minus the 'DstFPSema.isSaturated()' which is implied
```
It gives a bit of duplication (or at least similar code) but I think it's an
important disambiguation to make.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:1052
+ } else if (IsSigned && !DstFPSema.isSigned()) {
+ Value *Zero =
+ ConstantInt::get(CGF.getLLVMContext(), APInt(ResultWidth, 0));
----------------
`ConstantInt::getNullValue`?
================
Comment at: lib/Sema/Sema.cpp:537
+ case Type::STK_FixedPoint:
+ llvm_unreachable("Unknown cast from FixedPoint to boolean");
}
----------------
Will we want to support this?
Repository:
rC Clang
https://reviews.llvm.org/D50616
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
