This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC355462: [Fixed Point Arithmetic] Fixed Point and Integer
Conversions (authored by leonardchan, committed by ).
Changed pr
leonardchan updated this revision to Diff 189422.
leonardchan marked 11 inline comments as done.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56900/new/
https://reviews.llvm.org/D56900
Files:
clang/include/clang/AST/OperationKinds.def
clang/inc
leonardchan added inline comments.
Comment at: clang/test/Frontend/fixed_point_conversions.c:45
+
+short _Accum sa_const9 = 2u; // DEFAULT-DAG: @sa_const9 = {{.*}}global i16
256, align 2
+unsigned short _Accum usa_const3 = 2;
bjope wrote:
> Perhaps we should ver
bjope added inline comments.
Comment at: clang/test/Frontend/fixed_point_conversions.c:45
+
+short _Accum sa_const9 = 2u; // DEFAULT-DAG: @sa_const9 = {{.*}}global i16
256, align 2
+unsigned short _Accum usa_const3 = 2;
Perhaps we should verify that we do not me
bjope added inline comments.
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1223
if (SrcType->isFixedPointType()) {
-if (DstType->isFixedPointType()) {
- return EmitFixedPointConversion(Src, SrcType, DstType, Loc);
-} else if (DstType->isBooleanType()) {
+if (
rjmccall added a comment.
A couple tweaks, but otherwise LGTM.
Comment at: clang/include/clang/AST/OperationKinds.def:203
+/// CK_FixedPointToIntegral - Fixed point to an integral.
+///(int) 2.0k
This is super-picky, but please either just say "integral"
leonardchan added a comment.
@rjmccall @ebevhan @bjope *ping* any other comments on this patch?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56900/new/
https://reviews.llvm.org/D56900
___
cfe-commits mailing list
cf
leonardchan marked an inline comment as done.
leonardchan added inline comments.
Comment at: clang/test/Frontend/fixed_point_conversions.c:437
+ // DEFAULT-NEXT: [[RES:%[a-z0-9]+]] = trunc i39 [[SATMIN]] to i16
+ // DEFAULT-NEXT: store i16 [[RES]], i16* %sat_sa, align 2
+
-
leonardchan added inline comments.
Comment at: clang/test/Frontend/fixed_point_conversions.c:437
+ // DEFAULT-NEXT: [[RES:%[a-z0-9]+]] = trunc i39 [[SATMIN]] to i16
+ // DEFAULT-NEXT: store i16 [[RES]], i16* %sat_sa, align 2
+
ebevhan wrote:
> leonardchan wrote
leonardchan updated this revision to Diff 187706.
leonardchan marked an inline comment as done.
Herald added a subscriber: jdoerfert.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56900/new/
https://reviews.llvm.org/D56900
Files:
clang/include/clang/AST/Operatio
ebevhan added inline comments.
Comment at: clang/test/Frontend/fixed_point_conversions.c:437
+ // DEFAULT-NEXT: [[RES:%[a-z0-9]+]] = trunc i39 [[SATMIN]] to i16
+ // DEFAULT-NEXT: store i16 [[RES]], i16* %sat_sa, align 2
+
leonardchan wrote:
> leonardchan wrote
leonardchan marked an inline comment as done.
leonardchan added inline comments.
Comment at: clang/test/Frontend/fixed_point_conversions.c:437
+ // DEFAULT-NEXT: [[RES:%[a-z0-9]+]] = trunc i39 [[SATMIN]] to i16
+ // DEFAULT-NEXT: store i16 [[RES]], i16* %sat_sa, align 2
+
-
leonardchan marked an inline comment as done.
leonardchan added inline comments.
Comment at: clang/test/Frontend/fixed_point_conversions.c:437
+ // DEFAULT-NEXT: [[RES:%[a-z0-9]+]] = trunc i39 [[SATMIN]] to i16
+ // DEFAULT-NEXT: store i16 [[RES]], i16* %sat_sa, align 2
+
-
ebevhan added inline comments.
Comment at: clang/test/Frontend/fixed_point_conversions.c:437
+ // DEFAULT-NEXT: [[RES:%[a-z0-9]+]] = trunc i39 [[SATMIN]] to i16
+ // DEFAULT-NEXT: store i16 [[RES]], i16* %sat_sa, align 2
+
leonardchan wrote:
> ebevhan wrote:
>
leonardchan added a comment.
Oh I forgot to submit the comments.
Comment at: clang/lib/AST/ExprConstant.cpp:9825
+if (Result.isSigned() && !DstSigned) {
+ Overflow = Result < 0 || Result.ugt(DstMax);
+} else if (Result.isUnsigned() && DstSigned) {
leonardchan updated this revision to Diff 182945.
leonardchan marked 6 inline comments as done.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56900/new/
https://reviews.llvm.org/D56900
Files:
clang/include/clang/AST/OperationKinds.def
clang/include/clang/Basic
ebevhan added inline comments.
Comment at: clang/lib/AST/ExprConstant.cpp:9825
+if (Result.isSigned() && !DstSigned) {
+ Overflow = Result < 0 || Result.ugt(DstMax);
+} else if (Result.isUnsigned() && DstSigned) {
leonardchan wrote:
> ebevhan wrote:
leonardchan added inline comments.
Comment at: clang/lib/AST/ExprConstant.cpp:9825
+if (Result.isSigned() && !DstSigned) {
+ Overflow = Result < 0 || Result.ugt(DstMax);
+} else if (Result.isUnsigned() && DstSigned) {
ebevhan wrote:
> The `Result < 0
leonardchan updated this revision to Diff 182717.
leonardchan marked 6 inline comments as done.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56900/new/
https://reviews.llvm.org/D56900
Files:
clang/include/clang/AST/OperationKinds.def
clang/include/clang/Basic
ebevhan added inline comments.
Comment at: clang/lib/AST/ExprConstant.cpp:9825
+if (Result.isSigned() && !DstSigned) {
+ Overflow = Result < 0 || Result.ugt(DstMax);
+} else if (Result.isUnsigned() && DstSigned) {
The `Result < 0` is more clearly exp
rjmccall added inline comments.
Comment at: clang/lib/AST/ExprConstant.cpp:9839
+return Success(Result, E);
+ }
+
Can most of this reasonably be a method on `APFixedPoint`? (And likewise for
`CK_IntegralToFixedPoint`.) If nothing else, I suspect you are g
leonardchan updated this revision to Diff 182456.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56900/new/
https://reviews.llvm.org/D56900
Files:
clang/include/clang/AST/OperationKinds.def
clang/include/clang/Basic/FixedPoint.h
clang/lib/AST/Expr.cpp
clang
leonardchan created this revision.
leonardchan added reviewers: rjmccall, ebevhan, bjope.
leonardchan added a project: clang.
This patch includes the necessary code for converting between a fixed point
type and integer. This also includes constant expression evaluation for
conversions with these
23 matches
Mail list logo
