arsenm marked an inline comment as done.
arsenm added inline comments.

================
Comment at: test/SemaOpenCL/printf-format-strings.cl:65
+{
+    printf("%v4f\n", arg); // expected-warning {{format specifies type 'double 
__attribute__((ext_vector_type(4)))' but the argument has type 'float2' (vector 
of 2 'float' values)}}
 }
----------------
Anastasia wrote:
> arsenm wrote:
> > Anastasia wrote:
> > > arsenm wrote:
> > > > arsenm wrote:
> > > > > arsenm wrote:
> > > > > > Anastasia wrote:
> > > > > > > So there is no way to print vector of float? What will happen on 
> > > > > > > architectures that don't support doubles?
> > > > > > > 
> > > > > > > I guess it's the same for printf in general with the float type?
> > > > > > There is, it's converted to a vector of doubles. This case warns 
> > > > > > because the element count mismatches
> > > > > The conversion is to float if doubles aren't supported
> > > > The warning message is wrong though without doubles, and still says 
> > > > double.
> > > Can we add this explicitly to the test just for a record? I guess the 
> > > warning message can't be changed to print the type correctly?
> > > 
> > > Also would it make sense to test other builtin types: double, char...?
> > This should be easy to fix, but the way to test for doubles is enabled is 
> > making this unnecessarily difficult. It requires threading OpenCLOptions 
> > through all of the relevant functions to check for cl_khr_fp64. Why is this 
> > separate from LangOptions?
> I see. I think the problem is that it is set conditionally based on the 
> targets in earlier CL versions. So it had to be a separated into target 
> specific option. :(
Would it be reasonable to add a new HasNoDouble field or something in 
LangOptions?


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

https://reviews.llvm.org/D55023



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to