[PATCH] D115274: [IR][RFC] Memory region declaration intrinsic

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D115274#3432750 <https://reviews.llvm.org/D115274#3432750>, @lebedev.ri wrote:

> In D115274#3432740 <https://reviews.llvm.org/D115274#3432740>, @dtemirbulatov wrote:
>
>> Ping. Do you plan to commit the change? Any blockage?
>
> I don't know. I think i'm waiting for the dust to settle on the github pr debacle.

Any context on that? I feel like I missed something here...

Anyway, I find the overall wording here still a bit confusing. I would put more emphasis on the fact that this effectively restricts the "allocated object" to a certain offset range, which should have the following three effects:

- For non-inbounds GEPs, this should have no effect.
- For inbounds GEPs, if the GEP goes outside the range `[ptr+begin_offset, ptr+end_offset]`, the GEP result is poison.
- For accesses, if the access is outside the range `[ptr+begin_offset, ptr+end_offset-1]`, the behavior is undefined.

The current wording mostly emphasizes the middle point, but not so much the first and the last. And the fact that the "one past the end of the region" address is only valid for GEP inbounds but not for accesses is probably important for optimization purposes.



================
Comment at: llvm/docs/LangRef.rst:20841
+pointer, which is the first argument, must belong to the same address space
+as the argument. The second argument specifies the offset to the pointer (the
+first argument) at which the memory region begins. The third argument specifies
----------------
nit: "offset to" -> "offset from"?


================
Comment at: llvm/include/llvm/IR/Intrinsics.td:1196
+    [LLVMMatchType<0> /*ptr*/, llvm_i64_ty /*begin_offset*/, llvm_i64_ty /*end_offset*/],
+    [IntrNoMem, IntrSpeculatable, ReadNone<ArgIndex<0>>]>;
+
----------------
nit: Is the ReadNone here actually meaningful if the whole intrinsic is already IntrNoMem?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115274/new/

https://reviews.llvm.org/D115274