[CIR][LowerToLLVM] Lowered LLVM code for pointer arithmetic should have inbounds
Fix issue in https://github.com/llvm/clangir/issues/952.
Thanks for your time working on this!
The current changes are not related to ClangIR and it's not an appropriate way to resolve #952 . Given the following input code:
void foo(int *iptr) { iptr + 2; }
The CIR generated for the above code would be something similar to:
// ...
%1 = cir.const 2 : i32
%2 = cir.ptr_stride(%0 : !cir.ptr<i32>, %1 : i32), !cir.ptr<i32>
// ...
The generated CIR is further lowered to the following LLVM dialect code in clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp:
%0 = llvm.getelementptr %1[%2] : (!llvm.ptr, i32) -> !llvm.ptr, i32
Apparently the root cause is that LowerToLLVM fails to add the inbounds attribute to the llvm.getelementptr operation. You should update code in LowerToLLVM.cpp accordingly to fix this problem.
Alright, I'll work on LowerToLLVM to address this issue. In fact, I'm not entirely sure when the inbounds attribute should be added. I examined the code in Value *emitPointerArithmetic from clang/lib/CodeGen/CGExprScalar.cpp, but I couldn't identify a clear pattern. Could you please provide some guidance?
@liusy58 The getelementptr instruction is actually emitted in the CodeGenFunction::EmitCheckedInBoundsGEP function defined in the file you pointed out. It has two overloads, and you can find that both overloads set the inbounds flag without many prior conditions.
The C++ standard says that if the result of pointer arithmetic is out of bounds, the behavior is undefined. So I believe for cir.ptr_stride, you should always add the inbounds attribute to the lowered llvm.getelementptr operation.
Thank you. Let me check it.
Thanks @Lancern and @seven-mile for the great review and clarifications. @liusy58 welcome to the ClangIR project!
@Lancern Hi, I have update the code and could you please review it?
Hi, @seven-mile , I have updated the commit, please review it. Thanks!
#886 is a related potential area for follow-up work here if you're interested :)