Add constexpr support for accessing vector elements

[RKSimon]

Building on #76615 (#46593) by @Destroyerrrocket - we don't currently support accessing vector elements in constant expressions:

typedef int __v4si __attribute__((__vector_size__(16)));

constexpr  int extract0(__v4si v) {
  return v[0];
}
constexpr __v4si insert(__v4si v, int i, int s) {
    v[i] = s;
    return v;
}

https://gcc.godbolt.org/z/zfKs5EjWc has more examples (non-constant element indices, implicit conversions etc.)

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Simon Pilgrim (RKSimon)

Building on #76615 (#46593) by @Destroyerrrocket - we don't currently support accessing vector elements in constant expressions: ```cpp typedef int __v4si __attribute__((__vector_size__(16))); constexpr int extract0(__v4si v) { return v[0]; } constexpr __v4si insert(__v4si v, int i, int s) { v[i] = s; return v; } ``` https://gcc.godbolt.org/z/zfKs5EjWc has more examples (non-constant element indices, implicit conversions etc.)

[sethp]

I mentioned this deep in a discussion on the other PR, but for visibility there's some prior work on this over here: https://github.com/llvm/llvm-project/pull/72607 . I don't think it'd handle the assignment case (insert), but I think that change would work for extract0.

[Destroyerrrocket]

I'll be taking this issue. If someone else needs this asap and is willing to work in it, I'll pass whatever I've done to said person. Otherwise, I'd expect this to be done by june aprox.

[RKSimon]

@Destroyerrrocket Have you been able to look at this at all please?

[Destroyerrrocket]

Hey! Sorry, I had personal stuff going on this summer. I don't have much to contribute as of now, when I took a look it seemed like the clear next step was to extend the lvalue logic to stop it from trying to treat the vectors as pointers to vectors. If you want to pick this up, I did not write any relevant code so far. I'll probably come back to this at some point otherwise. I don't want to block any actually important progress by squatting this issue, that's for sure.

El dom, 22 sept 2024, 16:06, Simon Pilgrim @.***> escribió:

@Destroyerrrocket https://github.com/Destroyerrrocket Have you been able to look at this at all please?

— Reply to this email directly, view it on GitHub https://github.com/llvm/llvm-project/issues/87369#issuecomment-2366805606, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGBMPSEFOVO2SVJDQRDJBBLZX3FGVAVCNFSM6AAAAABFTXLQFKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRWHAYDKNRQGY . You are receiving this because you were mentioned.Message ID: @.***>

[Destroyerrrocket]

After some calendar analysis and having a look at my other projects, I suspect that I won't take another look at this at the very least until winter, sorry @RKSimon! But I don't want to set expectations I'm unlikely to meet right now...

[tbaederr]

This works: https://gcc.godbolt.org/z/dW59o83dz

(Just had to make the first parameter of the insert0 a reference) Which seems correct?

[Destroyerrrocket]

That is fantastic! Someone must have fixed this (maybe indirectly?) then; this can be closed. Thank you for checking in @tbaederr

[RKSimon]

Any idea what commit actually fixed this (and whether it was on purpose)?

[RKSimon]

https://gcc.godbolt.org/z/hdW8M8qz1 doesn't seem to need the reference 🤔

[Destroyerrrocket]

Might be a good idea to git bisect to find the fixing commit and give proper attribution. I can do that after I eat lunch, shouldn't take too much of my time.

The code you shared seems fine to me @RKSimon, Am I missing something? It is just copying the vector on the parameter (sorry, I'm on my phone and I might have missed something)

[Destroyerrrocket]

(I'll finish the git bisect later, 10 hops to go)

[RKSimon]

The code you shared seems fine to me @RKSimon, Am I missing something? It is just copying the vector on the parameter (sorry, I'm on my phone and I might have missed something)

I was referring to @tbaederr comment about it needing to be passed by reference - that doesn't seem to be necessary

[tbaederr]

Yeah it's only necessary because I didn't assign the result of insert0.

[Destroyerrrocket]

It was @vikramRH! Thank you! It was at commit 77534291fcbd2c784c54e39a60895e4f60f19742 Addressing this explicitly. So this can be closed knowing this was an intended fix! It should also be assigned to them.