rjmccall added inline comments.
================
Comment at: clang/lib/CodeGen/CGExprComplex.cpp:613
+ result = EmitUnpromotion(promotionTy, E->getSubExpr()->getType(), result);
+ return result;
+}
----------------
zahiraam wrote:
> rjmccall wrote:
> > You should unpromote only if we're not in a promoted context, which is to
> > say, only if the `PromotionType` that was passed in is null.
> oh! right. The promotionTy is not even used in the function. Thanks.
I'm sorry, but this is not the change I requested; I meant you need to do this:
```
QualType promotionTy = PromotionType.isNull() ?
getPromotionType(E->getType()) : PromotionType;
ComplexPairTy result = VisitMinus(E, promotionTy);
if (PromotionType.isNull())
result = EmitUnpromotion(promotionTy, result);
return result;
```
The fact that you weren't seeing problems because of this makes me concerned.
================
Comment at: clang/test/CodeGen/X86/Float16-complex.c:1184
+// X86-NEXT: store float [[NEG_I]], ptr [[RETVAL_IMAGP]], align 2
+// X86-NEXT: [[TMP0:%.*]] = load <2 x half>, ptr [[RETVAL]], align 2
+// X86-NEXT: ret <2 x half> [[TMP0]]
----------------
This code pattern is definitely wrong, and it's a sign that the expression
evaluator returned the wrong type. This is coercing a `_Complex float` into a
`_Complex _Float16` through memory, which is essentially reinterpreting the
first float as a pair of `_Float16`s. You should go through your tests and
make sure you don't see other instances of this.
================
Comment at: clang/test/Sema/Float16.c:13
-
-#ifdef HAVE
_Complex _Float16 a;
----------------
I don't know why these test changes are in this patch. My understanding is
that we already committed a patch to enable `_Float16`, and this patch is just
implementing the specialized emitter to avoid intermediate rounding when
AVX512FP16 isn't available.
In the review of that other patch, I argued why this is the wrong change for
this test.
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
