AmrDeveloper wrote: > I have concerns about the CIR representation here. I think we should be > aligning our representation of complex operations with the MLIR complex > dialect. As such, we want `__real__` to be lowerable to `complex.re` but the > explicit representation of intermediate pointer loads and stores is going to > make that difficult. > > The test case you have in this PR makes it difficult to reason about what I'm > interested in here because it is dealing so explicitly with pointers. I'd > like to consider something more like this: > > ``` > double f(double _Complex a, double _Complex b) { > return __real__ a + __real__ b; > } > ``` > > The incubator will generate the following CIR for this case: > > ``` > cir.func dso_local @_Z1fCdS_(%arg0: !cir.complex<!cir.double>, %arg1: > !cir.complex<!cir.double> -> !cir.double extra(#fn_attr) { > %0 = cir.alloca !cir.complex<!cir.double>, > !cir.ptr<!cir.complex<!cir.double>>, ["a", init] {alignment = 8 : i64} > %1 = cir.alloca !cir.complex<!cir.double>, > !cir.ptr<!cir.complex<!cir.double>>, ["b", init] {alignment = 8 : i64} > %2 = cir.alloca !cir.double, !cir.ptr<!cir.double>, ["__retval"] > {alignment = 8 : i64} > cir.store %arg0, %0 : !cir.complex<!cir.double>, > !cir.ptr<!cir.complex<!cir.double>> > cir.store %arg1, %1 : !cir.complex<!cir.double>, > !cir.ptr<!cir.complex<!cir.double>> > %3 = cir.complex.real_ptr %0 : !cir.ptr<!cir.complex<!cir.double>> -> > !cir.ptr<!cir.double> > %4 = cir.load align(8) %3 : !cir.ptr<!cir.double>, !cir.double > %5 = cir.complex.real_ptr %1 : !cir.ptr<!cir.complex<!cir.double>> -> > !cir.ptr<!cir.double> > %6 = cir.load align(8) %5 : !cir.ptr<!cir.double>, !cir.double > %7 = cir.binop(add, %4, %6) : !cir.double > cir.store %7, %2 : !cir.double, !cir.ptr<!cir.double> > %8 = cir.load %2 : !cir.ptr<!cir.double>, !cir.double > cir.return %8 : !cir.double > } > ``` > > That's not what I'd like to see. What I'd like to see is something more like > this: > > ``` > cir.func dso_local @_Z1fCdS_(%arg0: !cir.complex<!cir.double>, %arg1: > !cir.complex<!cir.double> -> !cir.double extra(#fn_attr) { > %0 = cir.alloca !cir.complex<!cir.double>, > !cir.ptr<!cir.complex<!cir.double>>, ["a", init] {alignment = 8 : i64} > %1 = cir.alloca !cir.complex<!cir.double>, > !cir.ptr<!cir.complex<!cir.double>>, ["b", init] {alignment = 8 : i64} > %2 = cir.alloca !cir.double, !cir.ptr<!cir.double>, ["__retval"] > {alignment = 8 : i64} > cir.store %arg0, %0 : !cir.complex<!cir.double>, > !cir.ptr<!cir.complex<!cir.double>> > cir.store %arg1, %1 : !cir.complex<!cir.double>, > !cir.ptr<!cir.complex<!cir.double>> > %3 = cir.load align(8) %0 : !cir.ptr<!cir.complex<!cir.double>> -> > !cir.complex<!cir.double> > %4 = cir.complex.real %3 : !cir.complex<!cir.double> -> !cir.double > %5 = cir.load align(8) %1 : !cir.ptr<!cir.complex<!cir.double>> -> > !cir.complex<!cir.double> > %6 = cir.complex.real %5 : !cir.complex<!cir.double> -> !cir.double > %7 = cir.binop(add, %4, %6) : !cir.double > cir.store %7, %2 : !cir.double, !cir.ptr<!cir.double> > %8 = cir.load %2 : !cir.ptr<!cir.double>, !cir.double > cir.return %8 : !cir.double > } > ``` > > The idea is that the `__real__` operation acts directly on the complex value > rather than manipulating its memory representation. We'll still need to go to > the memory representation when it gets lowered to LLVM IR, but I would like > to keep it higher level until then. > > For your test case: > > ``` > void foo10() { > double _Complex c; > double *realPtr = &__real__ c; > } > ``` > > the expression `&__real__ c` is represented in the AST like this: > > ``` > `-UnaryOperator <col:21, col:31> 'double *' prefix '&' cannot overflow > `-UnaryOperator <col:22, col:31> 'double' lvalue prefix '__real' > cannot overflow > `-DeclRefExpr <col:31> '_Complex double' lvalue Var 0x10ed3bd8 > 'c' '_Complex double' > ``` > > Hopefully, we could represent the `__real__` operator with a > `cir.complex.real` operation as I've proposed and then use the `&` operator > to do something to get a pointer. I don't know exactly how that would look. > > This is, of course, something we'd want to change in the incubator first to > take advantage of the more comprehensive testing available there.
I thought `&__real__` is represented as one op in AST, but it makes more sense to describe it as ComplexRealOp, and support & op for it, i will try to do that in incubator and update this PR once i finish it https://github.com/llvm/llvm-project/pull/144235 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits