[clang] Ternary elementwise builtin operators treat constness differently than others

[ckoparkar]

Consider this code:

typedef float float3 __attribute__((ext_vector_type(3)));

typedef struct {
  float3 b;
} struct_float3;

float3 foo(float3 a, const struct_float3* hi) {
  return __builtin_elementwise_fma(a, a, hi->b.yyy);
}

float3 bar(float3 a, const struct_float3* hi) {
  return __builtin_elementwise_pow(a, hi->b.yyy);
}

Clang will report an error about foo but not bar:

source.cpp:19:36: error: arguments are of different types ('float3' (vector of 3 'float' values) vs 'const float3' (vector of 3 'float' values))
   19 |   return __builtin_elementwise_fma(a, a, hi->b.yyy);
      |                                    ^

The solution is to use ASTContext::hasSameUnqualifiedType() in BuiltinElementwiseTernaryMath similar to #141397. This was originally discovered by @tbaederr in #153572 (comment).

[ckoparkar]

Another interesting case:

typedef const float vector4cfloat __attribute__((__vector_size__(16)));
typedef float vector4float __attribute__((__vector_size__(16)));

vector4cfloat baz(vector4cfloat x, vector4float y, vector4float z) {
  return __builtin_elementwise_fma(x,y,z);
}

vector4cfloat qux(vector4cfloat x, vector4float y, vector4float z) {
  return __builtin_elementwise_pow(x,y);
}

An error is reported for both baz and qux. Is this the expected behavior?

source.cpp:8:36: error: arguments are of different types ('vector4cfloat'
      (vector of 4 'const float' values) vs 'vector4float' (vector of 4 'float' values))
    8 |   return __builtin_elementwise_fma(x,y,z);
      |                                    ^
source.cpp:12:36: error: arguments are of different types ('vector4cfloat'
      (vector of 4 'const float' values) vs 'vector4float' (vector of 4 'float' values))
   12 |   return __builtin_elementwise_pow(x,y);
      |                                    ^
2 errors generated.

Also this is after I made the change to BuiltinElementwiseTernaryMath, but this isn't surprising because even the binary op is not accepted in this case.

[tbaederr]

Yeah the two canonical types in this case are:

VectorType 0x7d74be257470 '__attribute__((__vector_size__(4 * sizeof(const float)))) const float' 4
`-QualType 0x7d74be1fe3f1 'const float' const
  `-BuiltinType 0x7d74be1fe3f0 'float'
--
VectorType 0x7d74be257560 '__attribute__((__vector_size__(4 * sizeof(float)))) float' 4
`-BuiltinType 0x7d74be1fe3f0 'float'

We probably should be comparing the element types and not the vector types themselves...?

[ckoparkar]

Yeah comparing the element types and the number of elements should work? Let me make that change and see how some examples behave.

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Chaitanya Koparkar (ckoparkar)

Consider this code:

typedef float float3 __attribute__((ext_vector_type(3)));

typedef struct {
  float3 b;
} struct_float3;

float3 foo(float3 a, const struct_float3* hi) {
  return __builtin_elementwise_fma(a, a, hi->b.yyy);
}

float3 bar(float3 a, const struct_float3* hi) {
  return __builtin_elementwise_pow(a, hi->b.yyy);
}

Clang will report an error about foo but not bar:

source.cpp:19:36: error: arguments are of different types ('float3' (vector of 3 'float' values) vs 'const float3' (vector of 3 'float' values))
   19 |   return __builtin_elementwise_fma(a, a, hi->b.yyy);
      |                                    ^

The solution is to use ASTContext::hasSameUnqualifiedType() in BuiltinElementwiseTernaryMath similar to #141397. This was originally discovered by @tbaederr in #153572 (comment).

[ckoparkar]

@tbaederr This idea is more invasive than I initially thought. I think we'll have to
update how canonical types for these vectors are computed. Can we do that? Also, do we want to?

I didn't realize this and just updated BuiltinElementwiseTernaryMath (code here), but this is not enough because vector4float and vector4cfloat are not considered equal anywhere else.

void baz(vector4cfloat x, vector4float y, vector4float z) {
   vector4cfloat a = __builtin_elementwise_fma(x,y,z); //OK
   vector4float b = __builtin_elementwise_fma(x,y,z); //error
}

[tbaederr]

So? Your changes only change the checking for the input arguments, don't they? You don't have to care about the assignment.

[ckoparkar]

That's fair. Alright, let me create a PR then.