rjmccall added a comment.
In D113107#3726496 <https://reviews.llvm.org/D113107#3726496>, @zahiraam wrote:
> I think I introduced a bug when replacing the VisitUnaryMinus/Plus/Imag/Real
> with VisitMinus/Plus/Imag/Real. Now this simple test case is failing in the
> non-promotion path (with the +avx512fp16).
>
> _Float16 _Complex MinusOp_c_c(_Float16 c) {
>
> return -c;
>
> }
>
> error: cannot compile this scalar expression yet
>
> return -c;
> ^~
>
> 1 error generated.
>
> The way I implemented it is that I have added the HANDLE_UNOP macro but I
> think that's wrong. I think I need to keep the VisitUnary* methods and fork
> out of them in the promotion path! Same for both Scalar and Complex Exprs.
> Your thoughts?
Right, you still need to declare `VisitUnary*` methods that the CRTP visitor
will call. But you can avoid the boilerplate of having separate methods by
just adding the `PromotionType` argument to `VisitUnary*` with a default
argument that's appropriate for the normal path:
llvm::Value *VisitUnaryMinus(const UnaryOperator *E, QualType PromotionType =
QualType());
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3140
+ return CGF.Builder.CreateFPExt(result, ConvertType(E->getType()));
+ }
+ return result;
----------------
zahiraam wrote:
> rjmccall wrote:
> > Please extract this block out as:
> >
> > ```
> > llvm::Value *EmitPromotedValue(llvm::Value *result, QualType PromotionType);
> > ```
> These changes you are proposing is when the argument of the unary __imag /
> __real is of type _Complex Float16. I would think that this new method
> EmitPromotedValue would be replacing the equivalent code in
> ComplexEmitter::EmitPromoted instead, not in the scalar emitter, right?
I probably mixed up which emitter I commented on. The upshot is that I would
like there to be `EmitPromotedValue` and `EmitUnpromotedValue` helper functions
on both emitters (which of course would take/return an `llvm::Value*` on the
scalar emitter and a `CGComplexPair` on the complex emitter), just so that we
have all the value promotion/unpromotion logic for each emitter in one place.
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
