[IR] Add llvm.structured.gep instruction

[Keenuts]

DO NOT MERGE: we still need to agree on the instruction semantic. For now, this PR only contains the documentation, and the actual instruction implementation should be added before this can move forward.

Link to the RFC: https://discourse.llvm.org/t/rfc-adding-instructions-to-to-carry-gep-type-traversal-information/

...

This commit adds the llvm.structured.gep instruction to LLVM.

[Flakebi]

I guess this will be an intrinsic rather than an instruction?
So, something like @llvm.returnaddress and the other intrinsics in this document.
Would make sense to move it there.

[Keenuts]

Right make sense will move this in the other section, also forgot to change the title to Intrinsic vs Instruction.
For the instruction vs the intrinsic, I asked @nikic about this, and seems like the intrinsic is the way to go for such addition.

[Flakebi]

Thanks for the updates! A few more comments inline

[Flakebi]

This should use correct intrinsic syntax (now that we decided that it’s an intrinsic ;))
Reading through others, I think we should use the elementtype attribute:

declare ptr @llvm.structured.gep(ptr elementtype(<basetype>) <source>{, [i32/i64] <index> }*)

[Flakebi]

I think we should also copy from getelementptr here.

The type of each index argument depends on the type it is indexing into. When indexing into a (optionally packed) structure, only i32 integer constants are allowed (when using a vector of indices they must all be the same i32 integer constant). When indexing into an array, pointer or vector, integers of any width are allowed, and they are not required to be constant. These integers are treated as signed values where relevant.

Important points:

• Indices into structs must be constants (I think we can remove the language about vectors)
• Indices can be signed where relevant (😉)

I think signedness is important for some cases. We can be explicit and make only the first index into runtime arrays signed. I don’t think it makes sense in any other place (same as for getelementptr).
E.g. the following would be valid: call ptr @llvm.structured.gep(ptr elementtype([0 x <2 x i32>) %ptr, i32 -5, i32 1).

[Keenuts]

I'll add the note about constant indices on structs (it's there, but burried in the semantic section).

For the signedness, I'm not sure:
For SPIR-V, OpAccessChain indices are indeed signed, but only positive values are used for logical AFAIK.

We can be explicit and make only the first index into runtime arrays signed

This assumes you can get a pointer to an element in the middle of a runtime array and still derive a pointer from the wider array using this element address. Not sure if that's something we need, and it opens a bunch of complications.
What would that be useful for when doing logical accesses? (That cannot be done with positive indices only)

[Flakebi]

This assumes you can get a pointer to an element in the middle of a runtime array and still derive a pointer from the wider array using this element address.

Yes, almost all input languages (graphics SPIR-V may be an exception, I don’t know the details there) allow to have a pointer and move from there by adding/subtracting (e.g. int *p = &i; p += 1;).

Not being able to do that may pose severe limits the usefulness of structured.gep. I see that structured.gep can have use-cases with these limitations, but I hope we can make it generic enough, so that it can be used in other cases as well.

[Keenuts]

Isn't this something that can be solved at the codegen level? If this language is doing pointer arithmetic (assuming a subset) but still needs to target an architecture where pointer arithmetic cannot be represented outside of a logic addressing, I'd guess the FE should do:

• consider int *p to be struct { int *array_base, int offset }, and p += 1 to be sugar for p.offset += 1 or p.offset -= 1.
  Thus, the SGEP would be generated as: sgep [0 x type ], ptr p.array_base, i32 p.offset.

Let's look at this code:

void bar(int *ptr) { *ptr = 13 };

void foo(int *ptr) {
    *ptr = 12;
    bar(ptr - 1);
}

void entry(int *runtime_array) {
    foo(&runtime_array[3])
}

This will be lowered to something like:

void bar(ptr %ptr) {
  store i32 13, ptr %ptr
}

void foo(ptr %ptr) {
  store i32 12, ptr %ptr
  %1 = sgep [0 x i32] %ptr, i32 -1
  call bar(ptr %1)
}

void main(ptr %runtime_array) {
  %1 = sgep [0 x i32] %runtime_array, i32 3
  call foo(ptr %1)
}

Once inlined:

void main(ptr %runtime_array) {
  %1 = sgep [0 x i32] %runtime_array, i32 3
  store i32 12, ptr %1
  %2 = sgep [0 x i32] %1, i32 -1
  store i32 13, ptr %2
}

Optimizations wouldn't be allowed to change the second sgep into a sgep [0 x i32] %runtime_array, i32 2 because it's indexing into a logical array located at the pointer %1. Maybe in your architecture this is equivalent, but not always meaning target-agnostic optimizations wouldn't be allowed to change this.

But if this is handled in the FE, and pointers are represented as logical base + index, then you have this:

oid bar(ptr %ptr, i32 %index) {
  %1 = sgep [0 x i32] %ptr, i32 %index
  store i32 13, ptr %ptr
}

void foo(ptr %ptr, i32 %index) {
  %1 = sgep [0 x i32] %ptr, i32 %index
  store i32 12, ptr %1
  %2 = add i32 %index, i32 -1
  call bar(ptr %ptr, i32 %2)
}

void main(ptr %runtime_array, i32 %zero) {
  %1 = add i32 %zero, i32 3
  call foo(ptr %runtime_array, i32 %1)
}

void main(ptr %runtime_array, i32 %zero) {
  %1 = add %zero, i32 4
  %2 = sgep [0 x i32] %runtime_array, i32 %1
  store i32 12, ptr %2
  %3 = add %1, i32 -1
  %4 = sgep [0 x i32] %runtime_array, i32 %3
  store i32 13, ptr %4
}

This can now be optimized as we can compute the constant for each index, thus remove the add instruction in a target agnostic manner

[Flakebi]

Are you fine with allowing the end pointer (index=N) here or does that create problems for your intended uses?
I’m thinking of something like this:

Suggested change

	- If the traversed type is an array or vector of N elements with ``N > 0``,
	the index is assumed to belong to ``[0; N[``.
	- If the traversed type is an array or vector of N elements with ``N > 0``,
	the index is assumed to belong to ``[0; N]``.
	Note that creating a pointer to the end of an array (``index = N``) is allowed
	but loading or storing to this pointer is undefined behavior.

[Keenuts]

I'd rather not do this: even if we say accessing such pointer is invalid, we are implying either some kind of ordering, or defined behavior for out-of-bounds pointer calculations:

It would require the instruction either always return the same pointer for N, or return a pointer which can be compared to others in a significant way.
Is this request to support it = begin; it != end; it++ kind of case?

[Flakebi]

Is this request to support it = begin; it != end; it++ kind of case?

Yes, for feature parity with getelementptr.

[Keenuts]

@llvm-beanz @farzonl Is this OK as a semantic for you?