[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

This revision was automatically updated to reflect the committed changes. Closed by commit rL351364: [Fixed Point Arithmetic] Fixed Point Addition (authored by leonardchan, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D53738?vs=17686

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan added a comment. In D53738#1359135 , @rjmccall wrote: > I think that's the right direction, yeah. > > I thought I told you it was fine to commit this patch under that assumption > awhile ago. Did I just never click "accept"? Whoops. I don't

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. I think that's the right direction, yeah. I thought I told you it was fine to commit this patch under that assumption awhile ago. Did I just never click "accept"? Repository: rC Clang

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan added a comment. In D53738#1358249 , @rjmccall wrote: > In D53738#172 , @rjmccall wrote: > > > In D53738#1333276 , @leonardchan > > wrote: > > > > > In D5373

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added a comment. In D53738#172 , @rjmccall wrote: > In D53738#1333276 , @leonardchan > wrote: > > > In D53738#1326071 , @rjmccall > > wrote: > > > > > I'm fine

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added a comment. In D53738#1333276 , @leonardchan wrote: > In D53738#1326071 , @rjmccall wrote: > > > I'm fine with making this change under the assumption that we've gotten the > > language rule right. E

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan added a comment. In D53738#1326071 , @rjmccall wrote: > I'm fine with making this change under the assumption that we've gotten the > language rule right. Even if that weren't abstractly reasonable for general > language work — and I do thin

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added a comment. I'm fine with making this change under the assumption that we've gotten the language rule right. Even if that weren't abstractly reasonable for general language work — and I do think it's reasonable when we have a good-faith question about the right semantics — this i

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan added a comment. In D53738#1325998 , @rjmccall wrote: > We sent the question out, but we haven't gotten a response yet. I think > going forward under the idea that this just changes the type but does the > operation on the original operand t

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added a comment. We sent the question out, but we haven't gotten a response yet. I think going forward under the idea that this just changes the type but does the operation on the original operand types is the right way to go forward in the short term. Repository: rC Clang CHANGES

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan added a comment. In D53738#1320936 , @rjmccall wrote: > Okay, thanks, that makes sense to me. > > I'll ask around to find a way to contact the C committee. @rjmccall Any updates on this? If we aren't able to find a way to contact anyone affi

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added a comment. Okay, thanks, that makes sense to me. I'll ask around to find a way to contact the C committee. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53738/new/ https://reviews.llvm.org/D53738 ___

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1304 +RHSTy = ResultTy; + } + ebevhan wrote: > rjmccall wrote: > > ebevhan wrote: > > > rjmccall wrote: > > > > leonardchan wrote: > > > > > rjmccall wrote: > > > > > > leonardchan wr

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan updated this revision to Diff 176862. leonardchan marked an inline comment as done. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53738/new/ https://reviews.llvm.org/D53738 Files: clang/include/clang/AST/ASTContext.h clang/include/clang/AST/Type.h

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1304 +RHSTy = ResultTy; + } + rjmccall wrote: > ebevhan wrote: > > rjmccall wrote: > > > leonardchan wrote: > > > > rjmccall wrote: > > > > > leonardchan wrote: > > > > > > rjmccall wrote

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1304 +RHSTy = ResultTy; + } + ebevhan wrote: > rjmccall wrote: > > leonardchan wrote: > > > rjmccall wrote: > > > > leonardchan wrote: > > > > > rjmccall wrote: > > > > > > leonardchan w

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1304 +RHSTy = ResultTy; + } + rjmccall wrote: > leonardchan wrote: > > rjmccall wrote: > > > leonardchan wrote: > > > > rjmccall wrote: > > > > > leonardchan wrote: > > > > > > rjmccall w

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1304 +RHSTy = ResultTy; + } + leonardchan wrote: > rjmccall wrote: > > leonardchan wrote: > > > rjmccall wrote: > > > > leonardchan wrote: > > > > > rjmccall wrote: > > > > > > ebevhan w

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan marked an inline comment as done. leonardchan added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1304 +RHSTy = ResultTy; + } + rjmccall wrote: > leonardchan wrote: > > rjmccall wrote: > > > leonardchan wrote: > > > > rjmccall wrote: >

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1304 +RHSTy = ResultTy; + } + leonardchan wrote: > rjmccall wrote: > > leonardchan wrote: > > > rjmccall wrote: > > > > ebevhan wrote: > > > > > rjmccall wrote: > > > > > > leonardchan w

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan marked an inline comment as done. leonardchan added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1304 +RHSTy = ResultTy; + } + rjmccall wrote: > leonardchan wrote: > > rjmccall wrote: > > > ebevhan wrote: > > > > rjmccall wrote: > > >

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1304 +RHSTy = ResultTy; + } + leonardchan wrote: > rjmccall wrote: > > ebevhan wrote: > > > rjmccall wrote: > > > > leonardchan wrote: > > > > > rjmccall wrote: > > > > > > ebevhan wrote

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan marked an inline comment as done. leonardchan added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1304 +RHSTy = ResultTy; + } + rjmccall wrote: > ebevhan wrote: > > rjmccall wrote: > > > leonardchan wrote: > > > > rjmccall wrote: > > >

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1304 +RHSTy = ResultTy; + } + ebevhan wrote: > rjmccall wrote: > > leonardchan wrote: > > > rjmccall wrote: > > > > ebevhan wrote: > > > > > rjmccall wrote: > > > > > > Hmm. So adding a

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1304 +RHSTy = ResultTy; + } + rjmccall wrote: > leonardchan wrote: > > rjmccall wrote: > > > ebevhan wrote: > > > > rjmccall wrote: > > > > > Hmm. So adding a signed integer to an unsign

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1304 +RHSTy = ResultTy; + } + leonardchan wrote: > rjmccall wrote: > > ebevhan wrote: > > > rjmccall wrote: > > > > Hmm. So adding a signed integer to an unsigned fixed-point type alway

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan added inline comments. Comment at: clang/include/clang/Basic/FixedPoint.h:67 + FixedPointSemantics + getCommonSemantics(const FixedPointSemantics &Other) const; + rjmccall wrote: > leonardchan wrote: > > rjmccall wrote: > > > Actually representing t

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan updated this revision to Diff 176204. leonardchan marked 6 inline comments as done. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53738/new/ https://reviews.llvm.org/D53738 Files: clang/include/clang/AST/ASTContext.h clang/include/clang/AST/Type.h

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1304 +RHSTy = ResultTy; + } + ebevhan wrote: > rjmccall wrote: > > Hmm. So adding a signed integer to an unsigned fixed-point type always > > converts the integer to unsigned? That's

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1304 +RHSTy = ResultTy; + } + rjmccall wrote: > Hmm. So adding a signed integer to an unsigned fixed-point type always > converts the integer to unsigned? That's a little weird, but on

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added inline comments. Comment at: clang/include/clang/Basic/FixedPoint.h:67 + FixedPointSemantics + getCommonSemantics(const FixedPointSemantics &Other) const; + leonardchan wrote: > rjmccall wrote: > > Actually representing the fully-precise value is

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan updated this revision to Diff 175965. leonardchan marked 14 inline comments as done. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53738/new/ https://reviews.llvm.org/D53738 Files: clang/include/clang/AST/ASTContext.h clang/include/clang/AST/Type.h

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan added inline comments. Comment at: clang/include/clang/Basic/FixedPoint.h:67 + FixedPointSemantics + getCommonSemantics(const FixedPointSemantics &Other) const; + rjmccall wrote: > Actually representing the fully-precise value is operation-specific;

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:2638 + // type. + QualType getCorrespondingSignedFixedPointType(QualType Ty) const; + Please include in the comment here that, unlike `getCorrespondingUnsignedType`, this has to b

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan added a comment. > It's certainly interesting to degenerate integer-with-fixedpoint to just a > mul (since the scaling factor is 2^-n * 2^0, which is just 2^-n), but in the > general case you can't avoid doing the scale alignment. Unless I'm missing > something. No you're right. So

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added a comment. In D53738#1310212 , @leonardchan wrote: > In D53738#1309171 , @ebevhan wrote: > > > In D53738#1308314 , @leonardchan > > wrote: > > > > > > Generall

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan marked an inline comment as done. leonardchan added a comment. In D53738#1309171 , @ebevhan wrote: > In D53738#1308314 , @leonardchan > wrote: > > > > Generally I think it's good! One final note; I assu

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added a comment. In D53738#1308314 , @leonardchan wrote: > > 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? > >

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan marked an inline comment as done. leonardchan added a comment. > 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? Yes, but I imagine if we choose to keep t

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

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 cou

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan added a comment. @ebevhan Any more comments on this patch? 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

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan updated this revision to Diff 174640. leonardchan marked an inline comment as done. Repository: rC Clang https://reviews.llvm.org/D53738 Files: clang/include/clang/AST/ASTContext.h clang/include/clang/AST/Type.h clang/include/clang/Basic/FixedPoint.h clang/lib/AST/ASTContex

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan added inline comments. 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 + // UNSIG

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3385 + if (IsCommonSaturated) +CommonFixedSema.setSaturated(false); + ebevhan wrote: > Can EmitFixedPointConversion not determine this on its own? What I meant here was that rather

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan added a comment. > Good case to bring up. For addition, I think we just need to add an extra > condition that checks for unsigned padding in the result. Added this test > also. Actually this was wrong. Updated and instead dictate how the appropriate number of bits to `getCommonSema

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan updated this revision to Diff 174279. Repository: rC Clang https://reviews.llvm.org/D53738 Files: clang/include/clang/AST/ASTContext.h clang/include/clang/AST/Type.h clang/include/clang/Basic/FixedPoint.h clang/lib/AST/ASTContext.cpp clang/lib/Basic/FixedPoint.cpp clang

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan updated this revision to Diff 174235. leonardchan marked 13 inline comments as done. Repository: rC Clang https://reviews.llvm.org/D53738 Files: clang/include/clang/AST/ASTContext.h clang/include/clang/AST/Type.h clang/include/clang/Basic/FixedPoint.h clang/lib/AST/ASTConte

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan added a comment. In https://reviews.llvm.org/D53738#1299557, @ebevhan wrote: > I'd be interested in seeing tests for two saturating unsigned _Fract with and > without padding. > > If the padding case emits a uadd_sat, that seems wrong; uadd_sat doesn't > saturate on the padding bit,

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added a comment. I'd be interested in seeing tests for two saturating unsigned _Fract with and without padding. If the padding case emits a uadd_sat, that seems wrong; uadd_sat doesn't saturate on the padding bit, but will saturate the whole number, which can result in invalid represen

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan added inline comments. Comment at: clang/lib/Basic/FixedPoint.cpp:129 + std::max(NonFractBits, OtherNonFractBits) + CommonScale; + + bool ResultIsSigned = isSigned() || Other.isSigned(); ebevhan wrote: > Does this properly compensate for types o

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan updated this revision to Diff 174080. leonardchan marked 5 inline comments as done. Repository: rC Clang https://reviews.llvm.org/D53738 Files: clang/include/clang/AST/ASTContext.h clang/include/clang/AST/Type.h clang/include/clang/Basic/FixedPoint.h clang/lib/AST/ASTContex

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added a comment. > The only thing I didn't include in this patch were the suggested changes to > FixedPointSemantics which would indicate if the semantics were original from > an integer type. I'm not sure how necessary this is because AFAIK the only > time we would need to care if the

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan added a comment. For the integer conversion though, I was going to add that in a separate fixed-point-to-int and int-to-fixed-point patch. Repository: rC Clang https://reviews.llvm.org/D53738 ___ cfe-commits mailing list cfe-commits@

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan added a comment. Sorry for the delay in responding to this. Been working on a few other patches. A couple of the bigger changes so far: - Added `getCommonSemantics` to `FixedPointSemantics` for finding a common full precision semantic. - `getFixedPointSemantics` accepts an int type.

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan updated this revision to Diff 173961. leonardchan marked 11 inline comments as done. Repository: rC Clang https://reviews.llvm.org/D53738 Files: clang/include/clang/AST/ASTContext.h clang/include/clang/Basic/FixedPoint.h clang/lib/AST/ASTContext.cpp clang/lib/Basic/FixedPoi

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added a comment. In https://reviews.llvm.org/D53738#1288372, @ebevhan wrote: > In https://reviews.llvm.org/D53738#1288333, @rjmccall wrote: > > > In https://reviews.llvm.org/D53738#1287123, @ebevhan wrote: > > > > > In https://reviews.llvm.org/D53738#1284213, @rjmccall wrote: > > > > > >

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added a comment. In https://reviews.llvm.org/D53738#1288333, @rjmccall wrote: > In https://reviews.llvm.org/D53738#1287123, @ebevhan wrote: > > > In https://reviews.llvm.org/D53738#1284213, @rjmccall wrote: > > > > > Not out of line with other features that significantly break with what's

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added a comment. In https://reviews.llvm.org/D53738#1287123, @ebevhan wrote: > In https://reviews.llvm.org/D53738#1284213, @rjmccall wrote: > > > Not out of line with other features that significantly break with what's > > expressible. But the easy alternative to storing the intermedia

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added a comment. In https://reviews.llvm.org/D53738#1284213, @rjmccall wrote: > >> 2. The question is easily answered by pointing at the language spec. The > >> language does not say that the operands are promoted to a common type; it > >> says the result is determined numerically from

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added a comment. >> 1. You would have an inconsistency in either case, since e.g. numeric + >> otherwise always returns the same type as its operands, but this would not. > > True, but the CompoundAssignOperator already has that inconsistency with > ComputationResultType. Right, but i

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added a comment. In https://reviews.llvm.org/D53738#1283983, @rjmccall wrote: > In https://reviews.llvm.org/D53738#1283861, @ebevhan wrote: > > > In https://reviews.llvm.org/D53738#1283459, @rjmccall wrote: > > > > > Well, it could be passed around through most code as some sort of > > >

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added a comment. In https://reviews.llvm.org/D53738#1283861, @ebevhan wrote: > In https://reviews.llvm.org/D53738#1283459, @rjmccall wrote: > > > Well, it could be passed around through most code as some sort of > > abstractly-represented intermediate "type" which could be either a Qual

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added a comment. In https://reviews.llvm.org/D53738#1283459, @rjmccall wrote: > Well, it could be passed around through most code as some sort of > abstractly-represented intermediate "type" which could be either a QualType > or a fixed-point semantics. Sounds to me like you're descri

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added a comment. In https://reviews.llvm.org/D53738#1281894, @ebevhan wrote: > In https://reviews.llvm.org/D53738#1281332, @rjmccall wrote: > > > Well, maybe the cleanest solution would be to actually fold > > `CompoundAssignOperator` back into `BinaryOperator` and just allow > > `Bina

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added a comment. In https://reviews.llvm.org/D53738#1281332, @rjmccall wrote: > Well, maybe the cleanest solution would be to actually fold > `CompoundAssignOperator` back into `BinaryOperator` and just allow > `BinaryOperator` to optionally store information about the intermediate type

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added a comment. In https://reviews.llvm.org/D53738#1280193, @ebevhan wrote: > In https://reviews.llvm.org/D53738#1279069, @rjmccall wrote: > > > > The important difference would be that we separate the semantics of > > > performing the conversion and the operation. I understand that ad

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added a comment. In https://reviews.llvm.org/D53738#1279069, @rjmccall wrote: > > The important difference would be that we separate the semantics of > > performing the conversion and the operation. I understand that adding a new > > type could be a bit more involved than baking the con

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added a comment. In https://reviews.llvm.org/D53738#1278528, @ebevhan wrote: > In https://reviews.llvm.org/D53738#1278078, @rjmccall wrote: > > > I don't think we should add *types* just for this, but if you need to make > > a different kind of `BinaryOperator` that represents that the

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added a comment. In https://reviews.llvm.org/D53738#1278078, @rjmccall wrote: > I don't think we should add *types* just for this, but if you need to make a > different kind of `BinaryOperator` that represents that the semantics aren't > quite the same (the same way that the compound as

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added a comment. In https://reviews.llvm.org/D53738#1277172, @ebevhan wrote: > I want to float the idea again of adding an AST type to encapsulate an > arbitrary fixed-point semantic and using that as the 'common type' for binary > operations involving fixed-point types. This would ena

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

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, Common

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added a comment. I think I've already expressed that I'm not at all a fan of encoding the full-precision logic in the operations themselves and not explicitly in the AST. We already have (well, not yet, but we will) routines for emitting conversions to/from/between fixed-point types of

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3223-3247 +if (ResultWidth < CommonWidth) { + // In the event we extended the sign of both operands, the intrinsic will + // not saturate to the initial bit width of the result type. I

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

leonardchan created this revision. leonardchan added reviewers: ebevhan, bjope, rjmccall. leonardchan added a project: clang. This patch covers addition between fixed point types and other fixed point types or integers, using the conversion rules described in 4.1.4 of N1169. Usual arithmetic ru