zahiraam added a comment. In D113107#3554538 <https://reviews.llvm.org/D113107#3554538>, @rjmccall wrote:
> Hmm. I'm sorry, seeing the impact of this, I think it's not great to have > this `PromotionTy` field; it's too hard to get the invariants right. Let's do > this instead: > > 1. Add an `EmitPromoted` method to `ScalarExprEmitter`. The general rule for > this method is that it will generate a value of a promoted type, which should > be passed in. > 2. Make `EmitBinOps` take an optional promotion type. If it's provided, it > should call `EmitPromoted` instead of `Visit` to emit the operands, and it > should set up the promotion type as the formal type of the operation. > 3. In `EmitPromoted`, call `IgnoreParens` and then split into various cases: > - all the promotable floating-point arithmetic expressions (`BinAdd`, > `BinSub`, `BinMul`, `BinDiv`, and `UnaryMinus`), which should emit the > operands as promoted and then perform a promoted operation, like this: > > return Emit##OP(EmitBinOps(E, promotionTy)); > > - anything that promotion should propagate through, which under the > standard should not include assignments or casts, but could include things > like the comma operator and the ternary operator; > - everything else, which should emit the expression normally and then > promote the result. > 1. Change the standard path for the promotable arithmetic expressions (the > macro-expanded `VisitBinAdd` etc.) to detect the need to do a promoted > operation and then truncate, like this: > > QualType promotionTy = getPromotionType(E); > auto result = Emit##OP(EmitBinOps(E, promotionTy)); > if (result && promotionTy) result = Builder.CreateFPTrunc(result, > ConvertType(E->getType()), "unpromotion"); > return result; @rjmccall Thanks for that. I think I am ok for 1. 2. and 4. Still trying to figure out 3., EmitPromoted. When I make the change 4. the call to Emit##OP(EmitBinOps(E, promotionTy)); in VisitBinOP. Is this function still needed? Am i missing something? Also these changes seem to be only for ScalarExprEmitter not for ComplexExpEmitter. I still need a call to getPromotionType(E) for ComplexPairTy VisitBinAdd/Sub/Mul/Div , since there are also calling EmitBinOps, right? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
