[PATCH] D111985: [Clang] Add elementwise min/max builtins.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/test/Sema/builtins-elementwise-math.c:42
+  // expected-error at -1 {{argument types do not match, 'float4' (vector of 4 'float' values) != 'int3' (vector of 3 'int' values)}}
+}
----------------
aaron.ballman wrote:
> fhahn wrote:
> > aaron.ballman wrote:
> > > Additional tests I would like to see:
> > > ```
> > > int i;
> > > short s;
> > > 
> > > __builtin_elementwise_min(i, s); // ok (integer promotions)?
> > > 
> > > enum e { one, two };
> > > __builtin_elementwise_min(one, two); // ok (using enums)?
> > > 
> > > enum f { three };
> > > __builtin_elementwise_min(one, three); // ok (different enums)?
> > > 
> > > _ExtInt(32) ext;
> > > __builtin_elementwise_min(ext, ext); // ok (using bit-precise integers)?
> > > 
> > > const int ci;
> > > __builtin_elementwise_min(i, ci); // ok (qualifiers don't match)?
> > > ```
> > Thanks I'll add those!
> > 
> > at the moment `__builtin_elementwise_min(i, s); // ok (integer promotions)?` would be rejected, as per my response in Sema.
> > 
> > The other currently are not handled properly, which I'll fix!
> I'd still like to see the test case added so it's clear we intend to reject it. It may also be wise to add a test case involving an integer literal and a `short` variable cast to `int` to make it clear that you have to add casts sometimes to make this work.
> 
> Another interesting test is where sugars don't match. e.g.,
> ```
> int i;
> __attribute__((address_space(0))) int addr_space_i;
> typedef int foo;
> 
> __builtin_elementwise_min(i, addr_space_i); // ok (attributes don't match)?
> __builtin_elementwise_min(i, foo); // ok (sugar doesn't match)?
> ```
You might also need to handle something like:
```
int (i);
int j;
__builtin_elementwise_min(i, j);  // May get a type mismatch here
```
So you may need to do some type canonicalization to avoid these sorts of issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111985