arsenm added a comment. In D78862#2012560 <https://reviews.llvm.org/D78862#2012560>, @jdoerfert wrote:
> I think this is an improvement over the status quo and it looks fine to me. > > @arsenm I agree that we should tie this to the data layout (or at least > should try) but I guess there are open questions to answer and code to write. > I propose to accept this and work on the DL patch after. WDYT? Seems ok, but it's still burning an enum value which I guess isn't super important. With the datalayout property, we might really want the inverse attribute > > > In D78862#2008831 <https://reviews.llvm.org/D78862#2008831>, @manojgupta > wrote: > >> @nikic Thanks for the work. >> >> In D78862#2003684 <https://reviews.llvm.org/D78862#2003684>, @arsenm wrote: >> >> > FWIW I think this attribute should be replaced with a data layout >> > property, so this would eventually be removed >> >> >> @arsenm Is there any work planned on moving to data layout? Moving to data >> layout may affect cross TU inlining e.g. LTO where 1 TU is compiled with >> `-fno-delete-null-pointer-checks` and other TU is not. There might be other >> potential impact that we might not know yet. > > > If we link modules with mismatching data layouts we can/should deal with this > by utilizing more address spaces. That is, change the address space in one > module to a fresh one to keep the properties alive. There need to be rules > for this and infrastructure but something similar might be needed for > heterogeneous IR modules soon. Different story though. We can also combine > the attribute and the data layout if necessary, though I'm not a fan. This sounds really problematic and would require way more knowledge of target address spaces. I don't think this will work Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78862/new/ https://reviews.llvm.org/D78862 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits