Rebased

TODO: adapt to changes of get.active.lane.mask.

• rebased (onto clang-formatting commit for the SDA)
• DCE

Bug fix: DivLoopExit's loop may be nullptr

minor cleanup

NFC

• addressed reviewer comments, clean-up, tidy
• rebased

In D81083#2237284, @rsmith wrote:

Thanks for dropping by!

Have you considered using the ext_vector_type attribute instead of vector_size for this? That would have a couple of advantages:

• It's not a GCC attribute, so there's no risk that GCC would give different meaning to the same syntax (such as using one byte per vector element, which would be the natural meaning for a vector of bool as an extension of existing functionality).
• ext_vector_type takes as an argument a number of elements, not a size in bytes, so you could support vectors of non-multiple-of-8 bools.

Whichever solution is going to land in Clang, we will end up in the situation that Clang supports bool vectors and GCC does not. Any code that uses this feature and is supposed to compile with GCC will need some #ifdef guard. This is why i am not too worried about (ab-)using the vector_size attribute for this.

NFC. Make clang-tidy happy.

• made bool-vector layout target dependent (one byte per bool element on Hexagon, one bit everywhere else).
• Added hvx-specific debug info test.
• Updated docs.

The Hexagon builtins explicitly require bool vectors to have 8-bit wide bool elements. That clashes with our new vector type because bool vectors in builtin functions and vector_size bool vectors have the same internal representation in Clang. However, AFAIK Hexagon is the only target that actually need 8-bit bools. I see two possible ways out of this:

In D81083#2227760, @rsandifo-arm wrote:

I'm not qualified to review the CodeGen stuff (or accept the patch, obvs. :-)) but FWIW, here are some comments on the doc and Sema side.

Thanks for your comments! Maybe you know people who could review the CodeGen side of this..?

• Rebased.
• Allow comparisons on boolean vectors.
• Restored result type for vector comparisons on other types.
• Added operator, alignment and constexpr tests.

Please look into the lint comments. Otw, LGTM.

ping

• Fixed type printing & added type printing test.
• Rebased.

Some comments.

• It would be very helpful put the stress on the token value when it comes to the question which threads converge on the same static instruction. Does the token encode only the instance, does it include the static instruction 'id', ie can two instructions return the same token value?
• Not sure about llvm.experimental.constrained.anchor: ie, the current non-definition (it is the 'implementation-defined') invites in almost arbitrary semantics (depending on how the token value comes about).

Feel free to commit something like this without review

NFC. Cleanup.

• Fixed debug info representation for bool vectors (and added debug info test).
• Interpret 8*N with the N of vector_size(N) as the numbers of bool elements of the vector.

Nice! LGTM :)

Thanks for the feedback!
Planned change: interpret the N in vector_size(N) as the number of bytes in the type, such that, for example, a vector_size(2) bool vector holds 16 bits

This has been upstreamed for the most part.

NFC

• Style updates
• Rebased

Updated clang test

• Rebased
• Implement EmitFromMemory

Thanks for the comments so far.

Nit. LGTM otherwise.

Thanks!

I'd say you can commit a minor NFC like this one (in particular on a code base you have authored for the most part) without review.

Please consider the coding style suggestions (in particular for lower-casing function names). Otw, LGTM.

Basic Clang support for VE is now upstream: rG96d4ccf0

Removed getEHDataRegisterNumber.

In D81806#2097379, @sameerds wrote:

Do you still think that we should go ahead with this blunt fix? What is the ETA for the proper fix?

LGTM. Thanks!

In D81806#2094960, @sameerds wrote:

In D81806#2094869, @simoll wrote:

I am a bit surprised this is necessary. Actually, join divergence in bb3 should come about as a result of propagateLoopDivergence for the [bb1, bb2] loop and not immediately because it is a divergent exit.

I am not sure what you mean here. propagateLoopDivergence is calling propagateJoinDivergence ... so doesn't that count as a result of the former?

Also, a divergent loop exit does not automatically become join divergent, eg:

bb1: 
   %m.uni = <uni>
   %n.uni = <uni>
   br <uni>, %bb2, %bb3

bb2: [..]
   br <div>, %bb2, %bb3

bb3:
   %x.uni = phi [%m.uni, %bb2], [%n.uni %bb1]  <-- divergent loop exit that is not join divergent

But here, bb3 is not a join block for the bb2 loop, since there is only one path from the loop exit reaching bb3.

Exactly. With this patch %x.uni is marked as join divergent even though it is uniform: In its present form the patch solves one issue at the cost of precision.

I am a bit surprised this is necessary. Actually, join divergence in bb3 should come about as a result of propagateLoopDivergence for the [bb1, bb2] loop and not immediately because it is a divergent exit.
Also, a divergent loop exit does not automatically become join divergent, eg:

You could short cut this by calling propagateBranchDivergence right away instead of using DeferredTerminators (and mark the terminator as divergent). That's in the same spirit as calling pushUsers on a regular instruction since sync dependence is just a different kind of dependence.

LGTM. Thanks!

LGTM with one minor nit. Thanks!

Thanks!

Few nits.

Just one nit.

• Removed PredicatedInstruction, which isn't strictly necessary for this patch.
• Rebased.