zahiraam added a comment. In D113107#3736325 <https://reviews.llvm.org/D113107#3736325>, @rjmccall wrote:
> In D113107#3736312 <https://reviews.llvm.org/D113107#3736312>, @zahiraam > wrote: > >> This is a reduced test case from the codegen/complex-strictfp.c >> >> _Complex double g1, g2; >> double D; >> >> void foo(void) { >> >> g1 = g1 + D; >> >> } >> >> The issue is that we are calling in VisitBinAssign the function >> EmitUnpromotion (since promotionTy is null). This creates 2 constrained >> (-ffp-exception-behavior=maytrap used) fptrunc instructions. The verifier >> for Intrinsic::experimental_constrained_fptrunc fails at this check: >> https://github.com/intel/llvm/blob/sycl/llvm/lib/IR/Verifier.cpp#L5885 >> Is the call for EmitUnpromotion at the right place? Or should VisitBinAssign >> be treated the same way than the operator of HANDLEBINOP? Since in this case >> we are in the unpromoted path, we don't really need to add those fptrunc? >> Any thoughts? > > If `VisitBinAssign` isn't opting to promoted emission (by calling > `EmitPromoted...`), it shouldn't have to call `EmitUnpromotion`; it should be > able to rely on getting appropriate values for the type of its operands. > That's what I mean by a strong postcondition. So yeah, if you added a call > to `EmitUnpromotion` there as a workaround at some point, you should take it > out. If that breaks tests, it's because some emitter is leaking promoted > values out of the normal emission path, and we need to fix that emitter. Fixed codegen issue and LIT test issue mentioned by @rjmccall. I need second pair of eyes to check the resulting IR, @pengfei can you please help with that? Do we want to add the promoted path for VistiBinAssign in this patch or can we add it in a subsequent patch? Thanks. 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
