rjmccall added a comment. In D113107#3556886 <https://reviews.llvm.org/D113107#3556886>, @zahiraam wrote:
> 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? `VisitBin##OP`, which is the normal emission path, doesn't get a promotion type propagated into it. If you have an expression like `(a + 6*b) + c`, you'll be in this path when you visit the outermost `+`. In that path, you'll recognize that you should do promoted emission, so you'll call `EmitPromoted` on the sub-expressions, and then `EmitPromoted` will keep finding expressions with specialized handling for promotion (and therefore calling `EmitPromoted` recursively on their operands) until you get down to something that doesn't. The normal path will produce a `half`, but the promoted path will produce a `float`. > 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? Yeah, you'll do basically the same thing in `ComplexExprEmitter`. Oh, and I guess as part of that you'll want to be able to emit the conversion expressions between complex and scalar types (`_Imag` etc.) without truncating back to `half`, which means you'll need to add `EmitPromotedScalarExpr` and `EmitPromotedComplexExpr` methods to `CodeGenFunction`. 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
