rjmccall added inline comments.
================ Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3112 + result = CGF.Builder.CreateFPExt(result, ConvertType(E->getType())); + return result; +} ---------------- zahiraam wrote: > rjmccall wrote: > > The first version of the fallback path (the one in the `else`) is correct. > > Like with the complex emitter, though, you've got a bug where fall through > > from the earlier `switch` ends up in the second path; test case is > > something like `f16 + (true, f16)`. > > > > The best fix is to just not try to use `else` for your fallback path. The > > earlier blocks will return if they recognize cases that they can specially > > handle; everything should fall out, and you handle everything that reaches > > that point with the fallback path. > > > > This code should check for `UnaryOperator` like it does for > > `BinaryOperator` and delegate to your `_Real` / `_Imag` implementations > > above. You should probably also handle unary `+` and `-` so that using > > those does not cause intermediate truncations. > OK. Fixed the fallback part. But I can't find a test case that would make me > land into the switch for UnaryOperator . The code I added is not exercised. > Would you mind suggesting a test case? Not sure about the f16 + (true, f16) > above? > In general, you test the promoted path by using the operator in a context that wants to promote its operands. So you would test a unary operator by doing something like `f16 + -f16`: the operator is the operand of a Float16 `+`, which does promotion. 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
