aaron.ballman added inline comments.
================
Comment at: clang/lib/AST/Interp/Context.cpp:84-85
+ if (T->isAnyComplexType())
+ return std::nullopt;
+
----------------
tbaeder wrote:
> aaron.ballman wrote:
> > Hmmm, this seems surprising to me, I would have assumed that _Complex
> > needed to be a primitive type so that we could perform typical math on it
> > rather than treat it as a pair of values.
> I was going to add special opcodes for this case (or add //one// opcode for
> complex/vector/bitint/... things to do everything?).
>
> If we don't do it this way we'd have to add `isAnyComplexType()` checks
> everywhere so we can figure out if we need to look at one or two values,
> don't we?
Hmm I think it depends on whether we consider the type to be primitive or not,
maybe? I can see why it's not primitive -- it's comprised of two other
primitive types. So that could be reasonable. But it's also primitive in that
you need both of those values in order for the type to be meaningful for any
operation (e.g., we don't want to accidentally split the values; we're always
going to want to pass a complex value as "one thing", right). Buttttttt then
again, if we treat it as not being primitive, then perhaps that gives us more
optimization opportunities around things like "is an array of complex values an
array of structs or a struct of arrays" kind of situations where we can decide
to handle it differently depending on the situation.
So I guess I'm not certain of whether this is wrong or right.
================
Comment at: clang/test/AST/Interp/complex.cpp:7
+
+constexpr _Complex double z1 = {1.0, 2.0};
+static_assert(__real(z1) == 1.0);
----------------
tbaeder wrote:
> aaron.ballman wrote:
> > Can you add tests for `_Complex int` as well? (That's also covered by
> > `isAnyComplexType()`)
> There's a `FIXME` comment above in `EvalEmitter.cpp` so returning those
> doesn't work yet (but is otherwise not problematic I think.)
Hmmm, why the FIXME instead of just making the complex int type?
(Alternatively, should we not use `isAnyComplexType()` and wait to add complex
int support later?)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146408/new/
https://reviews.llvm.org/D146408
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
