andrew.w.kaylor added a comment.

I hope I'm not coming across as too argumentative here. I don't really have 
strong feelings about this function pointer type patch and ultimately I see 
that you are right, but the objections you are raising here would equally apply 
to what I'm working on with the IR linker and if I find a way to fix that, I'll 
care a bit more about that case. (If you'd like a preview, here's the bug I'm 
trying to fix: https://bugs.llvm.org/show_bug.cgi?id=38408)

You say "there is no semantic meaning for pointer types in LLVM IR" and that's 
fine, but there is a function PointerType::getElementType() and if I modify 
that function to always return the i8 type it will break a lot of things. So 
while there may be some sense in which the the pointer type cannot be the 
"correct" type, there is most definitely a sense in which it can be "incorrect" 
even if that sense isn't the strict semantic sense. I haven't looked at all the 
uses of  PointerType::getElementType() [or the related 
Type::getPointerElementType()]. I know a lot of them are just tests and 
assertions. Others are trivially walking through something that they know to be 
true by other means. A few seem (at least on first glance) to actually be doing 
something with it. For instance, llvm::getPointerStride() contains this line of 
code: "int64_t Size = DL.getTypeAllocSize(PtrTy->getElementType());"

I'm not arguing against opaque pointer types. I just feel like we're probably 
at least a couple of years away from having that. I'm also not arguing for 
robust and exhaustive correction of all cases where pointer types are not 
currently "working" in the sense that I am describing in my prior comments. I'm 
just saying that if someone runs into a specific case that is causing them 
problems and they submit a fix for that case, maybe we should let it through 
unless we have more of a reason than strict semantics rendering it unimportant.


https://reviews.llvm.org/D49403



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to